Make updater library thread safe

This was needed so that it could be called from Dart as well as flutter_main/C++.

It turns out flutter_main does not run on the "ui thread", so when Dart was calling
into the updater it would panic due to thinking the updater (which was using a thread local) was not yet initialized.

Also added the log-panics crate on Android so that panics appear in adb logcat.

example/counter/lib/main.dart

@@ -5,7 +5,8 @@ void main() {
   Updater.loadFlutterLibrary();
   var updater = Updater();
   // Just to prove the bindings work at all:
-  print(updater.activeVersion());
+  print("active version: ${updater.activeVersion()}");
+  print("active path: ${updater.activePath()}");
   runApp(const MyApp());
 }
 

example/counter/pubspec.lock

@@ -200,4 +200,4 @@ packages:
     source: hosted
     version: "2.1.4"
 sdks:
-  dart: ">=3.0.0-266.0.dev <4.0.0"
+  dart: ">=2.19.0 <4.0.0"

updater/cbindgen.toml

@@ -5,3 +5,16 @@ include_guard = "updater_h"
 autogen_warning = "/* Warning, this file is autogenerated by cbindgen. Don't modify this manually. */"
 cpp_compat = true
 line_length = 80
+
+# I don't know if these are required to export the shorebird_ symbols
+# since I've hit multiple levels of export trouble in libflutter.so.
+# But I'm leaving them here for now.
+after_includes = """
+#ifdef _WIN32
+#define SHOREBIRD_EXPORT __declspec(dllexport)
+#else
+#define SHOREBIRD_EXPORT __attribute__((visibility("default")))
+#endif
+"""
+[fn]
+prefix = "SHOREBIRD_EXPORT"

updater/dart_bindings/lib/src/bindings.dart

@@ -2,10 +2,8 @@
 // Probably https://pub.dev/packages/ffigen would work.
 
 import 'dart:ffi' as ffi;
-import 'dart:io' show Directory, Platform;
 
 import 'package:ffi/ffi.dart';
-import 'package:path/path.dart' as path;
 
 // This must be kept in sync with the C struct in updater.h.
 // Including *in the same order* as the C struct.
@@ -88,15 +86,22 @@ class UpdaterBindings {
   late GetVoid update;
 
   UpdaterBindings(this.library) {
-    init = library.lookupFunction<_SBInitFunc, SBInit>('shorebird_init');
-    activeVersion = library
-        .lookupFunction<_GetStringFunc, GetString>('shorebird_active_version');
-    activePath = library
-        .lookupFunction<_GetStringFunc, GetString>('shorebird_active_path');
-    freeString = library
-        .lookupFunction<_FreeStringFunc, FreeString>('shorebird_free_string');
-    checkForUpdate = library
-        .lookupFunction<_GetBoolFunc, GetBool>('shorebird_check_for_update');
-    update = library.lookupFunction<_GetVoidFunc, GetVoid>('shorebird_update');
+    // None of these call back into Dart, so they're all safely "isLeaf: true".
+    init = library.lookupFunction<_SBInitFunc, SBInit>('shorebird_init',
+        isLeaf: true);
+    activeVersion = library.lookupFunction<_GetStringFunc, GetString>(
+        'shorebird_active_version',
+        isLeaf: true);
+    activePath = library.lookupFunction<_GetStringFunc, GetString>(
+        'shorebird_active_path',
+        isLeaf: true);
+    freeString = library.lookupFunction<_FreeStringFunc, FreeString>(
+        'shorebird_free_string',
+        isLeaf: true);
+    checkForUpdate = library.lookupFunction<_GetBoolFunc, GetBool>(
+        'shorebird_check_for_update',
+        isLeaf: true);
+    update = library.lookupFunction<_GetVoidFunc, GetVoid>('shorebird_update',
+        isLeaf: true);
   }
 }

updater/library/Cargo.toml

@@ -21,8 +21,10 @@ serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.93"
 # Used for error handling.
 anyhow = {version = "1.0.69", features = ["backtrace"]}
-# For error!(), info!(), etc macros.
+# For error!(), info!(), etc macros. `print` will not show up on Android.
 log = "0.4.14"
+once_cell = "1.17.1"
 
 [target.'cfg(target_os = "android")'.dependencies]
 android_logger = "0.13.0"
+log-panics = { version = "2", features = ["with-backtrace"]}

updater/library/README.md

@@ -2,6 +2,61 @@
 
 The rust library that does the actual update work.
 
+## Design
+
+The updater library is built in Rust for safety (and modernity).  It's built
+as a C-compatible library, so it can be used from any language.
+
+The library is thread-safe, as it needs to be called both from the flutter_main
+thread (during initialization) and then later from the Dart/UI thread 
+(from application Dart code) in Flutter.
+
+The overarching principle with the Updater is "first, do no harm".  The updater
+should "fail open", terms of continuing to work with the currently installed
+or active version of the application even when the network is unavailable.
+
+The updater also needs to handle error cases conservatively, such as partial
+downloads from a server, or malformed responses (e.g. a proxy interfering)
+and not crash the application or leave the application in a broken state.
+
+Every time the updater runs it needs to verify that the currently installed
+patch is compatible with the currently installed base version.  If it is not,
+it should refuse to return paths to incompatible patches.
+
+The updater also needs to regularly verify that the current state directory
+is in a consistent state.  If it is not, it should invalidate any installed
+patches and return to a clean state.
+
+Not all of the above is implemented yet, but such is the intent.
+
+## Architecture
+
+The updater is split into separate layers.  The top layer is the C-compatible
+API, which is used by all consumers of the updater.  The C-compatible API
+is a thin wrapper around the Rust API, which is the main implementation but
+only used directly for testing (see the `cli` directory).
+
+Thread safety is handled by a global configuration object that is locked
+when accessed.  It's possible I've missed cases where this is not sufficient,
+and there could be thread safety issues in the library.
+
+* src/c_api.rs - C-compatible API
+* src/lib.rs - Rust API (and crate root)
+* src/update.rs - Core updater logic
+* src/config.rs - In memory configuration and thread locking
+* src/cache.rs - On-disk state management
+* src/logging.rs - Logging configuration (for platforms that need it)
+* src/network.rs - Logic dealing with network requests and updater server
+
+## Integration
+
+The updater library is built as a static library, and is linked into the
+libflutter.so as part of a custom build of Flutter.  We also link libflutter.so
+with the correct flags such that updater symbols are exposed to Dart.
+
+The `dart_bindings` directory contains the Dart bindings for the updater
+library.
+
 ## Building for Android
 
 The best way I found was to install:
@@ -18,6 +73,7 @@ rustup +beta target add \
 cargo +beta ndk --target aarch64-linux-android build --release
 ```
 
+## Development
 
 Uses cbindgen to generate the header file.
 

updater/library/include/updater.h

@@ -7,6 +7,12 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
+#ifdef _WIN32
+#define SHOREBIRD_EXPORT __declspec(dllexport)
+#else
+#define SHOREBIRD_EXPORT __attribute__((visibility("default")))
+#endif
+
 
 /**
  * Struct containing configuration parameters for the updater.
@@ -61,33 +67,33 @@ typedef struct AppParameters {
 extern "C" {
 #endif // __cplusplus
 
-void shorebird_init(const struct AppParameters *c_params);
+SHOREBIRD_EXPORT void shorebird_init(const struct AppParameters *c_params);
 
 /**
  * Return the active version of the app, or NULL if there is no active version.
  */
-char *shorebird_active_version(void);
+SHOREBIRD_EXPORT char *shorebird_active_version(void);
 
 /**
  * Return the path to the active version of the app, or NULL if there is no
  * active version.
  */
-char *shorebird_active_path(void);
+SHOREBIRD_EXPORT char *shorebird_active_path(void);
 
 /**
  * Free a string returned by the updater library.
  */
-void shorebird_free_string(char *c_string);
+SHOREBIRD_EXPORT void shorebird_free_string(char *c_string);
 
 /**
  * Check for an update.  Returns true if an update is available.
  */
-bool shorebird_check_for_update(void);
+SHOREBIRD_EXPORT bool shorebird_check_for_update(void);
 
 /**
  * Synchronously download an update if one is available.
  */
-void shorebird_update(void);
+SHOREBIRD_EXPORT void shorebird_update(void);
 
 #ifdef __cplusplus
 } // extern "C"

updater/library/src/config.rs

@@ -1,38 +1,38 @@
 // This file handles the global config for the updater library.
 
-use std::cell::RefCell;
+use std::sync::Mutex;
 
 use crate::updater::AppConfig;
+use once_cell::sync::OnceCell;
 
+// cbindgen looks for const, ignore these so it doesn't warn about them.
+
+/// cbindgen:ignore
 const DEFAULT_BASE_URL: &'static str = "https://shorebird-code-push-api-cypqazu4da-uc.a.run.app";
+/// cbindgen:ignore
 const DEFAULT_CHANNEL: &'static str = "stable";
 
-thread_local!(static CONFIG: RefCell<Option<ResolvedConfig>> = RefCell::new(None));
+fn global_config() -> &'static Mutex<ResolvedConfig> {
+    static INSTANCE: OnceCell<Mutex<ResolvedConfig>> = OnceCell::new();
+    INSTANCE.get_or_init(|| Mutex::new(ResolvedConfig::empty()))
+}
 
 pub fn with_config<F, R>(f: F) -> R
 where
     F: FnOnce(&ResolvedConfig) -> R,
 {
-    CONFIG
-        .try_with(|config| {
-            let config = config.borrow();
-            let config = config
-                .as_ref()
-                .expect("Must call updater_init before using the updater library.");
-            return f(config);
-        })
-        .expect("Must call updater_init before using the updater library.")
-}
+    let lock = global_config()
+        .lock()
+        .expect("Failed to acquire updater lock.");
 
-pub fn set_config(config: AppConfig) {
-    let config = resolve_config(config);
-    CONFIG.with(|c| {
-        let mut c = c.borrow_mut();
-        *c = Some(config);
-    });
+    if !lock.is_initialized {
+        panic!("Must call shorebird_init() before using the updater.");
+    }
+    return f(&lock);
 }
 
 pub struct ResolvedConfig {
+    is_initialized: bool,
     pub cache_dir: String,
     pub channel: String,
     pub client_id: String,
@@ -43,26 +43,43 @@ pub struct ResolvedConfig {
     pub base_url: String,
 }
 
-fn resolve_config(config: AppConfig) -> ResolvedConfig {
-    // Resolve the config
+impl ResolvedConfig {
+    pub fn empty() -> Self {
+        Self {
+            is_initialized: false,
+            cache_dir: String::new(),
+            channel: String::new(),
+            client_id: String::new(),
+            product_id: String::new(),
+            base_version: String::new(),
+            original_libapp_path: String::new(),
+            vm_path: String::new(),
+            base_url: String::new(),
+        }
+    }
+}
+
+pub fn set_config(config: AppConfig) {
     // If there is no base_url, use the default.
     // If there is no channel, use the default.
-    return ResolvedConfig {
-        client_id: config.client_id.to_string(),
-        base_url: config
-            .base_url
-            .as_deref()
-            .unwrap_or(DEFAULT_BASE_URL)
-            .to_owned(),
-        cache_dir: config.cache_dir.to_string(),
-        channel: config
-            .channel
-            .as_deref()
-            .unwrap_or(DEFAULT_CHANNEL)
-            .to_owned(),
-        product_id: config.product_id.to_string(),
-        base_version: config.base_version.to_string(),
-        original_libapp_path: config.original_libapp_path.to_string(),
-        vm_path: config.vm_path.to_string(),
-    };
+    let mut lock = global_config()
+        .lock()
+        .expect("Failed to acquire updater lock.");
+    lock.base_url = config
+        .base_url
+        .as_deref()
+        .unwrap_or(DEFAULT_BASE_URL)
+        .to_owned();
+    lock.channel = config
+        .channel
+        .as_deref()
+        .unwrap_or(DEFAULT_CHANNEL)
+        .to_owned();
+    lock.cache_dir = config.cache_dir.to_string();
+    lock.client_id = config.client_id.to_string();
+    lock.product_id = config.product_id.to_string();
+    lock.base_version = config.base_version.to_string();
+    lock.original_libapp_path = config.original_libapp_path.to_string();
+    lock.vm_path = config.vm_path.to_string();
+    lock.is_initialized = true;
 }

updater/library/src/logging.rs

@@ -1,13 +1,12 @@
 #[cfg(target_os = "android")]
 pub fn init_logging() {
-    use android_logger::Config;
-    use log::LevelFilter;
+    log_panics::init();
 
     android_logger::init_once(
-        Config::default()
+        android_logger::Config::default()
             // `flutter` tool ignores non-flutter tagged logs.
             .with_tag("flutter")
-            .with_max_level(LevelFilter::Debug),
+            .with_max_level(log::LevelFilter::Debug),
     );
     debug!("Logging initialized");
 }