feat: Add start_updater_thread to update off the main thread (#11)

* feat: Add start_updater_thread to update off the main thread

This makes it so that clients can easily not block when wanting to
queue an update.

I have a separate patch which updates the Engine to use this new
API.

I also needed to split the concept of the "next_boot" patch
from the "current_boot" patch, previously refered to as
"current" or "active" patch.  This required adding a
report_launch_start api to let the updater library know
when to set current_boot patch from next_boot.

I also removed the rust updater/cli in this as well as the
vmpath argument to init.

I also exposed the report_launch_success api, but its not yet
used by the Engine.

I renamed report_failed_launch to report_launch_failure to match
report_launch_start which I introduced.

* Update naming per comments from Felix.

Also added a helper for char* allocation (not sure if it's better).

* Update library/src/updater.rs

---------

Co-authored-by: Felix Angelov <felix@shorebird.dev>

Cargo.toml

@@ -1,2 +1,2 @@
 [workspace]
-members = ["cli", "library", "patch"]
+members = ["library", "patch"]

README.md

@@ -1,28 +1,27 @@
 # Updater library
 
-This is the C/Rust side of the Shorebird code push system.  This is built
+This is the Rust side of the Shorebird code push system.  This is built
 in Rust with a C API for easy calling from other languages, most notably
-for linking into libflutter.so.
-
-See cli/README.md for more documentation on the library.
+for linking into `libflutter.so`.
 
 ## Parts
-* cli: Test the updater library via the Rust API (for development).
-* dart_cli: Test ffi wrapping of updater library.
-* library: The rust library that does the actual update work.
-* dart_bindings: The Dart bindings for the updater library.
+* `dart_cli`: Test ffi wrapping of updater library.
+* `library`: The rust library that does the actual update work.
+* `dart_bindings`: The Dart bindings for the updater library.
 
 All of the interesting code is in the `library` directory.  There is also
-a README.md in that directory explaining the design.
+a [README.md](library/README.md) in that directory explaining the design.
 
 
 ## Developing
 
-It's best to edit this repository from within an engine checkout.  See [BUILDING_ENGINE.md](BUILDING_ENGINE.md) for instructions on how to set up an engine checkout.
+It's best to edit this repository from within an engine checkout.  See
+[BUILDING_ENGINE.md](BUILDING_ENGINE.md) for instructions on how to set up an
+engine checkout.
 
-The workflow I use involves 2-3 VSC windows:
+The workflow I use involves 2 to 3 VSC windows:
 
-1. Opening the engine 'src'.
+1. Opening the engine `src`.
 
 In that terminal I:
 ```
@@ -31,14 +30,22 @@ cd third_party/updater
 
 2. To build the updater as part of the engine:
 ```
-cargo ndk --target aarch64-linux-android build --release && ninja -C ../../out/android_release_arm64 && say "done"
+cargo ndk --target aarch64-linux-android build --release && \
+    ninja -C ../../out/android_release_arm64 && say "done"
 ```
 
-The cargo part *should not* be needed, but I haven't yet done the work to integrate the Rust code into the gn files for the Flutter engine yet.
+The cargo part *should not* be needed, but I haven't yet done the work to
+integrate the Rust code into the gn files for the Flutter engine yet.
 
-I add `say "done"` to the end as linking can take several minutes for release Android builds.
+I add `say "done"` to the end as linking can take several minutes for release
+Android builds.
 
-3.  In a second window, I open `code third_party/updater`.  I do this because otherwise the rust_analyzer can't seem to find the rust code.  We could fix this by adding the directory to the VSC workspace, but I'm not sure where we would put the workspace file in the first place.  `src` is actually shorebirdtech/buildroot and is controlled via gclient by shorebirdtech/engine/DEPS.
+3.  In a second window, I open `code third_party/updater`.  I do this because
+    otherwise the `rust_analyzer` can't seem to find the rust code.  We could
+    fix this by adding the directory to the VSC workspace, but I'm not sure
+    where we would put the workspace file in the first place.  `src` is actually
+    `shorebirdtech/buildroot` and is controlled via `gclient` by
+    `shorebirdtech/engine/DEPS`.
 
 4.  In a third window I open my test app. e.g.:
 
@@ -53,36 +60,21 @@ code .
 5. To run the test app with my local engine I use:
 
 ```
-flutter run --release  --local-engine-src-path ~/Documents/GitHub/engine/src --local-engine android_release_arm64
+shorebird run --local-engine-src-path $HOME/Documents/GitHub/engine/src \
+    --local-engine android_release_arm64
 ```
 
-You may also need to build `out/host_release` as `flutter` looks for some Dart .dill files there and if they're not there can fail to build.
+You may also need to build `out/host_release` once as `flutter build` looks for
+some  Dart  `.dill` files in `host_release`.
 
-## Note:
+## Coverage
 
-`--local-engine-src-path` doesn't work on ARM Macs at the moment due to: https://github.com/flutter/flutter/issues/124620
+We'd like to get to 100% coverage but aren't there yet.
 
-Alternatives being considered include
-* pushing artifacts to a dev server: https://github.com/shorebirdtech/shorebird/pull/277
-* Or we could build a local artifact proxy mode for `shorebird`, but would require teaching `flutter` how to use an alternative artifacts directory (or just making a new checkout of flutter) and telling it how to override the version in engine.version.  This is needed to make sure that gradel requests artifacts of a different version than existing so as to avoid poluting the gradle cache.
+https://github.com/taiki-e/cargo-llvm-cov
+is the best tool I've found for generating coverage reports.
 
-## Steps to use artifact via dev server.
+Install:
+https://github.com/taiki-e/cargo-llvm-cov#installation
 
-* Make changes to updater
-* Commit changes, save git hash.
-* Update engine/DEPS with git hash to your updater changes.  Commit and save engine hash.
-* Build engine using build_engine/build.sh
-* Upload built artifacts to dev servers using build_engine/upload.sh (currently need to modify upload.sh)
-* Create a change to artifact_proxy to include your proxy mapping, similar to: https://github.com/shorebirdtech/shorebird/pull/277
-* Land the change to artifact_proxy and wait for the dev proxy to update.
-* Change your shorebird/bin/cache/flutter/bin/internal/engine.version to your engine version.  commit.
-* Run FLUTTER_STORAGE_BASE_URL=https://artifact-proxy-kmdbqkx7rq-uc.a.run.app/ shorebird/bin/cache/flutter/bin/flutter clean
-* Change shorebird_cli to point to the dev server: https://artifact-proxy-kmdbqkx7rq-uc.a.run.app/ 
-* `shorebird` commands should now work with your artifacts.
-
-Once you've created a single unique engine revision, you don't have to create new ones so long as you know how to clear the flutter cache and gradle cache to make them re-download.
-
-Why can't we use a local artifact_proxy?  Gradle will complain if you use a non-https source.  You can change that on a per-project basis however:
-https://docs.gradle.org/7.5/dsl/org.gradle.api.artifacts.repositories.UrlArtifactRepository.html#org.gradle.api.artifacts.repositories.UrlArtifactRepository:allowInsecureProtocol
-
-This is clearly not our final dev workflow. 🤣
+`cargo llvm-cov` will then generate the report.

cli/Cargo.toml

@@ -1,10 +0,0 @@
-[package]
-name = "cli"
-version = "0.1.0"
-edition = "2021"
-
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
-[dependencies]
-clap = { version = "4.1.6", features = ["derive"] }
-updater = { path = "../library" }

cli/src/main.rs

@@ -1,65 +0,0 @@
-extern crate updater;
-
-use clap::{Parser, Subcommand};
-
-#[derive(Parser)]
-#[command(author, version, about, long_about = None, arg_required_else_help=true)]
-struct Cli {
-    #[command(subcommand)]
-    command: Option<Commands>,
-}
-
-#[derive(Subcommand)]
-enum Commands {
-    Check {},
-    Current {},
-    Update {},
-}
-
-fn main() {
-    let cli = Cli::parse();
-
-    let config = updater::AppConfig {
-        cache_dir: "updater_cache".to_owned(),
-        release_version: "0.1.0+1".to_owned(),
-        original_libapp_paths: vec!["libapp.so".to_owned()],
-    };
-    let yaml_str = "
-app_id: demo
-channel: stable
-base_url: http://localhost:8000
-";
-    updater::init(config, yaml_str).expect("init failed");
-
-    // You can check for the existence of subcommands, and if found use their
-    // matches just as you would the top level cmd
-    match &cli.command {
-        Some(Commands::Check {}) => {
-            let needs_update = updater::check_for_update();
-            println!("Checking for update...");
-            if needs_update {
-                println!("Update needed.");
-            } else {
-                println!("No update needed.");
-            }
-        }
-        Some(Commands::Current {}) => {
-            let version = updater::active_patch();
-            println!("Current version info:");
-            match version {
-                Some(v) => {
-                    println!("path: {:?}", v.path);
-                    println!("number: {:?}", v.number);
-                }
-                None => {
-                    println!("None");
-                }
-            }
-        }
-        Some(Commands::Update {}) => {
-            let status = updater::update();
-            println!("Update: {}", status);
-        }
-        None => {}
-    }
-}

dart_bindings/lib/src/bindings.dart

@@ -21,8 +21,6 @@ class AppParameters extends ffi.Struct {
   // ignore: non_constant_identifier_names
   external int original_libapp_paths_size;
   // ignore: non_constant_identifier_names
-  external ffi.Pointer<Utf8> vm_path;
-  // ignore: non_constant_identifier_names
   external ffi.Pointer<Utf8> cache_dir;
 
   static ffi.Pointer<AppParameters> allocate({
@@ -48,7 +46,6 @@ class AppParameters extends ffi.Struct {
     for (var i = 0; i < libappPaths.length; i++) {
       config.ref.original_libapp_paths[i] = libappPaths[i].toNativeUtf8();
     }
-    config.ref.vm_path = libflutterPath.toNativeUtf8();
     config.ref.cache_dir = cacheDir.toNativeUtf8();
     return config;
   }
@@ -63,7 +60,6 @@ class AppParameters extends ffi.Struct {
       calloc.free(config.ref.original_libapp_paths[i]);
     }
     calloc.free(config.ref.original_libapp_paths);
-    calloc.free(config.ref.vm_path);
     calloc.free(config.ref.cache_dir);
     calloc.free(config);
   }

library/include/updater.h

@@ -21,18 +21,14 @@
  */
 typedef struct AppParameters {
   /**
-   * version_name, required.  Named version of the app, off of which updates
-   * are based.  Can be either a version number or a hash.
+   * release_version, required.  Named version of the app, off of which
+   * updates are based.  Can be either a version number or a hash.
    */
-  const char *version_name;
-  /**
-   * version_code, required.  Integer version of the app, off of which
-   * updates are based.  Monotonically increasing on Android/Play Store.
-   */
-  long version_code;
+  const char *release_version;
   /**
    * Array of paths to the original aot library, required.  For Flutter apps
-   * these are the paths to the bundled libapp.so.  May be used for compression downloaded artifacts.
+   * these are the paths to the bundled libapp.so.  May be used for
+   * compression downloaded artifacts.
    */
   const char *const *original_libapp_paths;
   /**
@@ -61,13 +57,13 @@ void shorebird_init(const struct AppParameters *c_params,
 /**
  * Return the active patch number, or NULL if there is no active patch.
  */
-SHOREBIRD_EXPORT char *shorebird_active_patch_number(void);
+SHOREBIRD_EXPORT char *shorebird_next_boot_patch_number(void);
 
 /**
  * Return the path to the active patch for the app, or NULL if there is no
  * active patch.
  */
-SHOREBIRD_EXPORT char *shorebird_active_path(void);
+SHOREBIRD_EXPORT char *shorebird_next_boot_patch_path(void);
 
 /**
  * Free a string returned by the updater library.
@@ -84,12 +80,37 @@ SHOREBIRD_EXPORT bool shorebird_check_for_update(void);
  */
 SHOREBIRD_EXPORT void shorebird_update(void);
 
+/**
+ * Start a thread to download an update if one is available.
+ */
+SHOREBIRD_EXPORT void shorebird_start_update_thread(void);
+
+/**
+ * Tell the updater that we're launching from what it told us was the
+ * next patch to boot from.  This will copy the next_boot patch to be
+ * the current_boot patch.
+ * It is required to call this function before calling
+ * shorebird_report_launch_success or shorebird_report_launch_failure.
+ */
+SHOREBIRD_EXPORT void shorebird_report_launch_start(void);
+
 /**
  * Report that the app failed to launch.  This will cause the updater to
  * attempt to roll back to the previous version if this version has not
  * been launched successfully before.
  */
-SHOREBIRD_EXPORT void shorebird_report_failed_launch(void);
+SHOREBIRD_EXPORT void shorebird_report_launch_failure(void);
+
+/**
+ * Report that the app launched successfully.  This will mark the current
+ * as having been launched successfully.  We don't currently do anything
+ * with this information, but it could be used to record a point at which
+ * we will not roll back from.
+ * This is not currently wired up to be called from the Engine.  It's unclear
+ * where best to connect it.  Expo waits 5 seconds after the app launches
+ * and then marks the launch as successful.  We could do something similar.
+ */
+SHOREBIRD_EXPORT void shorebird_report_launch_success(void);
 
 #ifdef __cplusplus
 } // extern "C"

library/src/assets.rs

@@ -0,0 +1,122 @@
+// Modeled after AAssetManager from Android NDK
+
+use std::fmt::{Debug, Formatter};
+use std::io::{Read, Seek};
+
+/// The AssetProvider is a trait which allows the updater to load assets from
+/// different sources.
+pub struct AssetProvider {
+    ops: Box<dyn AssetProviderOps>,
+}
+
+impl Debug for AssetProvider {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("AssetProvider")
+            .field("ops", &"Box<dyn AssetProviderOps>")
+            .finish()
+    }
+}
+
+pub trait AssetProviderOps: Send + Sync + 'static {
+    fn open(&self, path: &str) -> Option<Asset>;
+}
+
+pub struct Asset {
+    ops: Box<dyn AssetOps>,
+}
+
+impl Asset {
+    pub fn new(ops: Box<dyn AssetOps>) -> Self {
+        Self { ops }
+    }
+}
+
+pub trait AssetOps: Read + Seek {
+    fn close(&mut self) {}
+}
+
+impl AssetProvider {
+    pub fn empty() -> Self {
+        Self {
+            ops: Box::new(EmptyAssetProviderOps {}),
+        }
+    }
+
+    pub fn new(ops: Box<dyn AssetProviderOps>) -> Self {
+        Self { ops }
+    }
+
+    pub fn open(&self, path: &str) -> Option<Asset> {
+        info!("AssetProvider::open({:?})", path);
+        self.ops.open(path)
+    }
+}
+
+impl Read for Asset {
+    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+        info!("Asset::read({:?})", buf);
+        self.ops.read(buf)
+    }
+}
+
+impl Seek for Asset {
+    fn seek(&mut self, pos: std::io::SeekFrom) -> std::io::Result<u64> {
+        info!("Asset::seek({:?})", pos);
+        self.ops.seek(pos)
+    }
+}
+
+impl Drop for Asset {
+    fn drop(&mut self) {
+        info!("Asset::drop()");
+        self.ops.close();
+    }
+}
+
+struct EmptyAssetProviderOps {}
+
+impl AssetProviderOps for EmptyAssetProviderOps {
+    fn open(&self, _path: &str) -> Option<Asset> {
+        info!("EmptyAssetProviderOps::open({:?})", _path);
+        None
+    }
+}
+
+// struct FileSystemAssetProviderOps {
+// }
+
+// impl AssetProviderOps for FileSystemAssetProviderOps {
+//     fn open(&self, path: &str) -> Option<Asset> {
+//         let file = std::fs::File::open(path);
+//         if file.is_err() {
+//             return None;
+//         }
+//         let file = file.unwrap();
+//         Some(Asset {
+//             ops: Box::new(FileSystemAssetOps { file }),
+//         })
+//     }
+// }
+
+// #[derive(Debug)]
+// struct FileSystemAssetOps {
+//     file: std::fs::File,
+// }
+
+// impl AssetOps for FileSystemAssetOps {
+//     fn close(&self, _asset: &Asset) {
+//         self.file.sync_all().unwrap();
+//     }
+// }
+
+// impl Read for FileSystemAssetOps {
+//     fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+//         self.file.read(buf)
+//     }
+// }
+
+// impl Seek for FileSystemAssetOps {
+//     fn seek(&mut self, pos: std::io::SeekFrom) -> std::io::Result<u64> {
+//         self.file.seek(pos)
+//     }
+// }

library/src/c_api.rs

@@ -14,16 +14,13 @@ use crate::updater;
 /// NOTE: If this struct is changed all language bindings must be updated.
 #[repr(C)]
 pub struct AppParameters {
-    /// version_name, required.  Named version of the app, off of which updates
-    /// are based.  Can be either a version number or a hash.
-    pub version_name: *const libc::c_char,
-
-    /// version_code, required.  Integer version of the app, off of which
-    /// updates are based.  Monotonically increasing on Android/Play Store.
-    pub version_code: libc::c_long,
+    /// release_version, required.  Named version of the app, off of which
+    /// updates are based.  Can be either a version number or a hash.
+    pub release_version: *const libc::c_char,
 
     /// Array of paths to the original aot library, required.  For Flutter apps
-    /// these are the paths to the bundled libapp.so.  May be used for compression downloaded artifacts.
+    /// these are the paths to the bundled libapp.so.  May be used for
+    /// compression downloaded artifacts.
     pub original_libapp_paths: *const *const libc::c_char,
 
     /// Length of the original_libapp_paths array.
@@ -33,10 +30,16 @@ pub struct AppParameters {
     pub cache_dir: *const libc::c_char,
 }
 
+/// Converts a C string to a Rust string, does not free the C string.
 fn to_rust(c_string: *const libc::c_char) -> String {
     unsafe { CStr::from_ptr(c_string).to_str().unwrap() }.to_string()
 }
 
+/// Converts a Rust string to a C string, caller must free the C string.
+fn allocate_c_string(rust_string: &str) -> *mut c_char {
+    CString::new(rust_string).unwrap().into_raw()
+}
+
 fn to_rust_vector(c_array: *const *const libc::c_char, size: libc::c_int) -> Vec<String> {
     let mut result = Vec::new();
     for i in 0..size {
@@ -51,11 +54,7 @@ fn app_config_from_c(c_params: *const AppParameters) -> updater::AppConfig {
 
     updater::AppConfig {
         cache_dir: to_rust(c_params_ref.cache_dir),
-        release_version: format!(
-            "{}+{}",
-            to_rust(c_params_ref.version_name),
-            c_params_ref.version_code
-        ),
+        release_version:to_rust(c_params_ref.release_version),
         original_libapp_paths: to_rust_vector(
             c_params_ref.original_libapp_paths,
             c_params_ref.original_libapp_paths_size,
@@ -82,12 +81,11 @@ pub extern "C" fn shorebird_init(c_params: *const AppParameters, c_yaml: *const
 
 /// Return the active patch number, or NULL if there is no active patch.
 #[no_mangle]
-pub extern "C" fn shorebird_active_patch_number() -> *mut c_char {
-    let patch = updater::active_patch();
+pub extern "C" fn shorebird_next_boot_patch_number() -> *mut c_char {
+    let patch = updater::next_boot_patch();
     match patch {
-        Some(v) => {
-            let c_patch = CString::new(v.number.to_string()).unwrap();
-            c_patch.into_raw()
+        Some(p) => {
+            allocate_c_string(&p.number.to_string())
         }
         None => std::ptr::null_mut(),
     }
@@ -96,13 +94,11 @@ pub extern "C" fn shorebird_active_patch_number() -> *mut c_char {
 /// Return the path to the active patch for the app, or NULL if there is no
 /// active patch.
 #[no_mangle]
-// rename to shorebird_patch_path
-pub extern "C" fn shorebird_active_path() -> *mut c_char {
-    let version = updater::active_patch();
-    match version {
-        Some(v) => {
-            let c_version = CString::new(v.path).unwrap();
-            c_version.into_raw()
+pub extern "C" fn shorebird_next_boot_patch_path() -> *mut c_char {
+    let patch = updater::next_boot_patch();
+    match patch {
+        Some(p) => {
+            allocate_c_string(&p.path)
         }
         None => std::ptr::null_mut(),
     }
@@ -131,12 +127,34 @@ pub extern "C" fn shorebird_update() {
     updater::update();
 }
 
+/// Start a thread to download an update if one is available.
+#[no_mangle]
+pub extern "C" fn shorebird_start_update_thread() {
+    updater::start_update_thread();
+}
+
+/// Tell the updater that we're launching from what it told us was the
+/// next patch to boot from.  This will copy the next_boot patch to be
+/// the current_boot patch.
+/// It is required to call this function before calling
+/// shorebird_report_launch_success or shorebird_report_launch_failure.
+#[no_mangle]
+pub extern "C" fn shorebird_report_launch_start() {
+    let result = updater::report_launch_start();
+    match result {
+        Ok(_) => {}
+        Err(e) => {
+            error!("Error recording launch start: {:?}", e);
+        }
+    }
+}
+
 /// Report that the app failed to launch.  This will cause the updater to
 /// attempt to roll back to the previous version if this version has not
 /// been launched successfully before.
 #[no_mangle]
-pub extern "C" fn shorebird_report_failed_launch() {
-    let result = updater::report_failed_launch();
+pub extern "C" fn shorebird_report_launch_failure() {
+    let result = updater::report_launch_failure();
     match result {
         Ok(_) => {}
         Err(e) => {
@@ -144,3 +162,21 @@ pub extern "C" fn shorebird_report_failed_launch() {
         }
     }
 }
+
+/// Report that the app launched successfully.  This will mark the current
+/// as having been launched successfully.  We don't currently do anything
+/// with this information, but it could be used to record a point at which
+/// we will not roll back from.
+/// This is not currently wired up to be called from the Engine.  It's unclear
+/// where best to connect it.  Expo waits 5 seconds after the app launches
+/// and then marks the launch as successful.  We could do something similar.
+#[no_mangle]
+pub extern "C" fn shorebird_report_launch_success() {
+    let result = updater::report_launch_success();
+    match result {
+        Ok(_) => {}
+        Err(e) => {
+            error!("Error recording launch success: {:?}", e);
+        }
+    }
+}

library/src/cache.rs

@@ -4,7 +4,6 @@ use std::fs::File;
 use std::io::{BufReader, BufWriter};
 use std::path::{Path, PathBuf};
 
-use anyhow::Ok;
 use serde::{Deserialize, Serialize};
 
 use crate::updater::UpdateError;
@@ -49,8 +48,10 @@ pub struct UpdaterState {
     /// List of patches that successfully booted. We will never rollback past
     /// one of these for this device.
     successful_patches: Vec<usize>,
-    /// Currently selected slot.
-    current_slot_index: Option<usize>,
+    /// Slot that the app is currently booted from.
+    current_boot_slot_index: Option<usize>,
+    /// Slot that will be used for next boot.
+    next_boot_slot_index: Option<usize>,
     /// List of slots.
     slots: Vec<Slot>,
     // Add file path or FD so modifying functions can save it to disk?
@@ -61,7 +62,8 @@ impl UpdaterState {
         Self {
             cache_dir,
             release_version,
-            current_slot_index: None,
+            current_boot_slot_index: None,
+            next_boot_slot_index: None,
             latest_downloaded_patch: None,
             failed_patches: Vec::new(),
             successful_patches: Vec::new(),
@@ -143,20 +145,32 @@ impl UpdaterState {
         Ok(())
     }
 
-    /// This is NOT the current booted path (we don't keep that in memory yet).
-    /// This is the patch that is selected in the state.json, which may or may
-    /// not be the one that is booted, but will be the one used next boot.
-    pub fn current_patch(&self) -> Option<PatchInfo> {
-        if self.slots.is_empty() {
+    fn patch_info_at(&self, index: usize) -> Option<PatchInfo> {
+        if index >= self.slots.len() {
             return None;
         }
-        if let Some(slot_index) = self.current_slot_index {
-            if slot_index >= self.slots.len() {
-                return None;
-            }
-            let slot = &self.slots[slot_index];
-            // Otherwise return the version info from the current slot.
-            return Some(slot.to_patch_info());
+        let slot = &self.slots[index];
+        Some(slot.to_patch_info())
+    }
+
+    /// This is the current patch that is running.
+    /// Will be None if:
+    /// - There was no good patch at time of boot.
+    /// - The updater has been initialized but no boot recorded yet.
+    pub fn current_boot_patch(&self) -> Option<PatchInfo> {
+        if let Some(slot_index) = self.current_boot_slot_index {
+            return self.patch_info_at(slot_index)
+        }
+        None
+    }
+
+    /// This is the patch that will be used for the next boot.
+    /// Will be None if:
+    /// - There has never been a patch selected.
+    /// - There was a patch selected but it was later marked as bad.
+    pub fn next_boot_patch(&self) -> Option<PatchInfo> {
+        if let Some(slot_index) = self.next_boot_slot_index {
+            return self.patch_info_at(slot_index)
         }
         None
     }
@@ -196,7 +210,7 @@ impl UpdaterState {
     }
 
     pub fn activate_latest_bootable_patch(&mut self) -> Result<(), UpdateError> {
-        self.set_current_slot(self.latest_bootable_slot());
+        self.set_next_boot_patch_slot(self.latest_bootable_slot());
         self.save().map_err(|_| UpdateError::FailedToSaveState)
     }
 
@@ -205,7 +219,10 @@ impl UpdaterState {
         if self.slots.is_empty() {
             return 0;
         }
-        if let Some(slot_index) = self.current_slot_index {
+        if let Some(slot_index) = self.current_boot_slot_index {
+            // This does not check next_boot_slot_index, we're assuming that
+            // whoever is calling this is OK with replacing the next boot
+            // patch.
             if slot_index == 0 {
                 return 1;
             }
@@ -272,7 +289,7 @@ impl UpdaterState {
                 patch_number: patch.number,
             },
         );
-        self.set_current_slot(Some(slot_index));
+        self.set_next_boot_patch_slot(Some(slot_index));
 
         if (self.latest_downloaded_patch.is_none())
             || (self.latest_downloaded_patch.unwrap() < patch.number)
@@ -287,10 +304,23 @@ impl UpdaterState {
         self.save()?;
         Ok(())
     }
-
-    pub fn set_current_slot(&mut self, maybe_index: Option<usize>) {
-        self.current_slot_index = maybe_index;
+    
+    /// Sets the current_boot slot to the next_boot slot.
+    pub fn activate_current_patch(&mut self) -> Result<(), UpdateError> {
+        if  self.next_boot_slot_index.is_none() {
+            return Err(UpdateError::InvalidState("No patch to activate.".to_owned()));
+        }
+        self.current_boot_slot_index = self.next_boot_slot_index.clone();
+        assert!(self.current_boot_slot_index.is_some());
+        Ok(())
     }
+
+    /// Switches the next boot slot to the given slot or clears it if None.
+    pub fn set_next_boot_patch_slot(&mut self, maybe_index: Option<usize>) {
+        self.next_boot_slot_index = maybe_index;
+    }
+
+    /// Returns highest patch number that has been downloaded for this release.
     pub fn latest_patch_number(&self) -> Option<usize> {
         self.latest_downloaded_patch
     }
@@ -318,15 +348,15 @@ mod tests {
     }
 
     #[test]
-    fn current_patch_does_not_crash() {
+    fn next_boot_patch_does_not_crash() {
         let tmp_dir = TempDir::new("example").unwrap();
         let mut state = test_state(&tmp_dir);
-        assert_eq!(state.current_patch(), None);
-        state.current_slot_index = Some(3);
-        assert_eq!(state.current_patch(), None);
+        assert_eq!(state.next_boot_patch(), None);
+        state.next_boot_slot_index = Some(3);
+        assert_eq!(state.next_boot_patch(), None);
         state.slots.push(super::Slot::default());
         // This used to crash, where index was bad, but slots were not empty.
-        assert_eq!(state.current_patch(), None);
+        assert_eq!(state.next_boot_patch(), None);
     }
 
     #[test]

library/src/config.rs

@@ -41,7 +41,6 @@ pub struct ResolvedConfig {
     pub app_id: String,
     pub release_version: String,
     pub original_libapp_paths: Vec<String>,
-    pub vm_path: String,
     pub base_url: String,
 }
 
@@ -55,7 +54,6 @@ impl ResolvedConfig {
             app_id: String::new(),
             release_version: String::new(),
             original_libapp_paths: Vec::new(),
-            vm_path: String::new(),
             base_url: String::new(),
         }
     }

library/src/updater.rs

@@ -11,7 +11,7 @@ use std::fs;
 use std::io::{Cursor, Read, Seek};
 use std::path::{Path, PathBuf};
 
-use anyhow::{Context, Result};
+use anyhow::{Context};
 
 pub enum UpdateStatus {
     NoUpdate,
@@ -381,40 +381,60 @@ where
     Ok(())
 }
 
-/// Reads the current patch from the cache and returns it.
-pub fn active_patch() -> Option<PatchInfo> {
+/// The patch which will be run on next boot (which may still be the same
+/// as the current boot).
+/// This may be changed any time update() or start_update_thread() are called.
+pub fn next_boot_patch() -> Option<PatchInfo> {
     return with_config(|config| {
         let state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version);
-        return state.current_patch();
+        return state.next_boot_patch();
     });
 }
 
+/// The patch which is currently booted.  This is None until
+/// report_launch_start() is called at which point it is copied from
+/// next_boot_patch.
+pub fn current_boot_patch() -> Option<PatchInfo> {
+    return with_config(|config| {
+        let state = UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version);
+        return state.current_boot_patch();
+    });
+}
+
+pub fn report_launch_start() -> anyhow::Result<()> {
+    with_config(|config| {
+        let mut state =
+            UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version);
+        // Validate that we have an installed patch.
+        // Make that patch the "booted" patch.
+        state.activate_current_patch()?;
+        state.save()
+    })
+}
+
 /// Report that the current active path failed to launch.
 /// This will mark the patch as bad and activate the next best patch.
-pub fn report_failed_launch() -> Result<(), UpdateError> {
+pub fn report_launch_failure() -> Result<(), UpdateError> {
     info!("Reporting failed launch.");
     with_config(|config| {
         let mut state =
             UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version);
 
-        // FIXME: We need to separate out the concept of "running patch" and
-        // "next patch to activate".  Currently these are smooshed which will
-        // make async updates impossible to implement.
         let patch = state
-            .current_patch()
+            .current_boot_patch()
             .ok_or(UpdateError::InvalidState("No current patch".to_string()))?;
         state.mark_patch_as_bad(&patch);
         state.activate_latest_bootable_patch()
     })
 }
 
-pub fn report_successful_launch() -> Result<(), UpdateError> {
+pub fn report_launch_success() -> Result<(), UpdateError> {
     with_config(|config| {
         let mut state =
             UpdaterState::load_or_new_on_error(&config.cache_dir, &config.release_version);
 
         let patch = state
-            .current_patch()
+            .current_boot_patch()
             .ok_or(UpdateError::InvalidState("No current patch".to_string()))?;
         state.mark_patch_as_good(&patch);
         state.save().map_err(|_| UpdateError::FailedToSaveState)
@@ -436,6 +456,19 @@ pub fn update() -> UpdateStatus {
     });
 }
 
+/// This does not return status.  The only output is the change to the saved
+/// cache. The Engine calls this during boot and it will check for an update
+/// and install it if available.
+pub fn start_update_thread() {
+    // This holds the lock on the config for the entire duration of the update
+    // call which is wrong. We should be able to release the lock during the
+    // network requests.
+    std::thread::spawn(move || {
+        let status = update();
+        info!("Update thread finished with status: {}", status);
+    });
+}
+
 #[cfg(test)]
 mod tests {
     use std::fs;
@@ -479,13 +512,13 @@ mod tests {
         let tmp_dir = TempDir::new("example").unwrap();
         init_for_testing(&tmp_dir);
         assert_eq!(
-            crate::report_failed_launch(),
+            crate::report_launch_failure(),
             Err(crate::UpdateError::InvalidState(
                 "No current patch".to_string()
             ))
         );
         assert_eq!(
-            crate::report_successful_launch(),
+            crate::report_launch_success(),
             Err(crate::UpdateError::InvalidState(
                 "No current patch".to_string()
             ))
@@ -518,15 +551,16 @@ mod tests {
                 .expect("move failed");
             state.save().expect("save failed");
         });
-        assert!(crate::active_patch().is_some());
+        assert!(crate::next_boot_patch().is_some());
         // pretend we booted from it
-        crate::report_successful_launch().unwrap();
-        assert!(crate::active_patch().is_some());
+        crate::report_launch_start().unwrap();
+        crate::report_launch_success().unwrap();
+        assert!(crate::next_boot_patch().is_some());
         // mark it bad.
-        crate::report_failed_launch().unwrap();
+        crate::report_launch_failure().unwrap();
         // Technically might need to "reload"
         // ask for current patch (should get none).
-        assert!(crate::active_patch().is_none());
+        assert!(crate::next_boot_patch().is_none());
     }
 
     #[test]