fix: shutdown order (#3073)

* fix: race condition in Daemon.stop()

* fix: do not stop Daemon on context leave

* fix: remove duplicate Daemons.term calls

* feat: honor dependency order when shutting terminating Daemons

* fixes, and remove started

---------

Co-authored-by: Aiden McClelland <me@drbonez.dev>

container-runtime/src/Adapters/EffectCreator.ts

@@ -289,6 +289,7 @@ export function makeEffects(context: EffectContext): Effects {
     getStatus(...[o]: Parameters<T.Effects["getStatus"]>) {
       return rpcRound("get-status", o) as ReturnType<T.Effects["getStatus"]>
     },
+    /// DEPRECATED
     setMainStatus(o: { status: "running" | "stopped" }): Promise<null> {
       return rpcRound("set-main-status", o) as ReturnType<
         T.Effects["setHealth"]

container-runtime/src/Adapters/RpcListener.ts

@@ -292,10 +292,13 @@ export class RpcListener {
         )
       })
       .when(stopType, async ({ id }) => {
-        this.callbacks?.removeChild("main")
         return handleRpc(
           id,
-          this.system.stop().then((result) => ({ result })),
+          this.system.stop().then((result) => {
+            this.callbacks?.removeChild("main")
+
+            return { result }
+          }),
         )
       })
       .when(exitType, async ({ id, params }) => {

container-runtime/src/Adapters/Systems/SystemForStartOs.ts

@@ -70,20 +70,13 @@ export class SystemForStartOs implements System {
       this.starting = true
       effects.constRetry = utils.once(() => effects.restart())
       let mainOnTerm: () => Promise<void> | undefined
-      const started = async (onTerm: () => Promise<void>) => {
-        await effects.setMainStatus({ status: "running" })
-        mainOnTerm = onTerm
-        return null
-      }
       const daemons = await (
         await this.abi.main({
           effects,
-          started,
         })
       ).build()
       this.runningMain = {
         stop: async () => {
-          if (mainOnTerm) await mainOnTerm()
           await daemons.term()
         },
       }

core/startos/src/service/effects/subcontainer/sync.rs

@@ -53,16 +53,22 @@ pub fn kill_init(procfs: &Path, chroot: &Path) -> Result<(), Error> {
                     )
                 })?;
                 if pids.0.len() == 2 && pids.0[1] == 1 {
-                    nix::sys::signal::kill(Pid::from_raw(pid), nix::sys::signal::SIGKILL)
+                    match nix::sys::signal::kill(
-                        .with_ctx(|_| {
+                        Pid::from_raw(pid),
-                            (
+                        Some(nix::sys::signal::SIGKILL),
-                                ErrorKind::Filesystem,
+                    ) {
-                                lazy_format!(
+                        Err(Errno::ESRCH) => Ok(()),
-                                    "kill pid {} (determined to be pid 1 in subcontainer)",
+                        a => a,
-                                    pid
+                    }
-                                ),
+                    .with_ctx(|_| {
-                            )
+                        (
-                        })?;
+                            ErrorKind::Filesystem,
+                            lazy_format!(
+                                "kill pid {} (determined to be pid 1 in subcontainer)",
+                                pid
+                            ),
+                        )
+                    })?;
                 }
             }
         }
@@ -510,10 +516,13 @@ pub fn exec(
     std::thread::spawn(move || {
         if let Ok(pid) = recv_pid.blocking_recv() {
             for sig in sig.forever() {
-                nix::sys::signal::kill(
+                match nix::sys::signal::kill(
                     Pid::from_raw(pid),
                     Some(nix::sys::signal::Signal::try_from(sig).unwrap()),
-                )
+                ) {
+                    Err(Errno::ESRCH) => Ok(()),
+                    a => a,
+                }
                 .unwrap();
             }
         }

core/startos/src/service/transition/mod.rs

@@ -29,7 +29,25 @@ impl ServiceActorSeed {
     pub fn start(&self) -> Transition<'_> {
         Transition {
             kind: TransitionKind::Starting,
-            future: self.persistent_container.start().boxed(),
+            future: async {
+                self.persistent_container.start().await?;
+                let id = &self.id;
+                self.ctx
+                    .db
+                    .mutate(|db| {
+                        db.as_public_mut()
+                            .as_package_data_mut()
+                            .as_idx_mut(id)
+                            .or_not_found(id)?
+                            .as_status_info_mut()
+                            .started()
+                    })
+                    .await
+                    .result?;
+
+                Ok(())
+            }
+            .boxed(),
         }
     }
 
@@ -47,8 +65,7 @@ impl ServiceActorSeed {
                             .as_idx_mut(id)
                             .or_not_found(id)?
                             .as_status_info_mut()
-                            .as_started_mut()
+                            .stopped()
-                            .ser(&None)
                     })
                     .await
                     .result?;

core/startos/src/status/mod.rs

@@ -28,11 +28,24 @@ impl StatusInfo {
     }
 }
 impl Model<StatusInfo> {
+    pub fn start(&mut self) -> Result<(), Error> {
+        self.as_desired_mut().map_mutate(|s| Ok(s.start()))?;
+        Ok(())
+    }
+    pub fn started(&mut self) -> Result<(), Error> {
+        self.as_started_mut()
+            .map_mutate(|s| Ok(Some(s.unwrap_or_else(|| Utc::now()))))?;
+        Ok(())
+    }
     pub fn stop(&mut self) -> Result<(), Error> {
         self.as_desired_mut().map_mutate(|s| Ok(s.stop()))?;
         self.as_health_mut().ser(&Default::default())?;
         Ok(())
     }
+    pub fn stopped(&mut self) -> Result<(), Error> {
+        self.as_started_mut().ser(&None)?;
+        Ok(())
+    }
     pub fn init(&mut self) -> Result<(), Error> {
         self.as_started_mut().ser(&None)?;
         self.as_desired_mut().map_mutate(|s| {

sdk/base/lib/Effects.ts

@@ -67,7 +67,7 @@ export type Effects = {
     packageId?: PackageId
     callback?: () => void
   }): Promise<StatusInfo>
-  /** indicate to the host os what runstate the service is in */
+  /** DEPRECATED: indicate to the host os what runstate the service is in */
   setMainStatus(options: SetMainStatus): Promise<null>
 
   // dependency

sdk/base/lib/types.ts

@@ -44,10 +44,7 @@ export namespace ExpectedExports {
    * This is the entrypoint for the main container. Used to start up something like the service that the
    * package represents, like running a bitcoind in a bitcoind-wrapper.
    */
-  export type main = (options: {
+  export type main = (options: { effects: Effects }) => Promise<DaemonBuildable>
-    effects: Effects
-    started(onTerm: () => PromiseLike<void>): PromiseLike<null>
-  }) => Promise<DaemonBuildable>
 
   /**
    * Every time a service launches (both on startup, and on install) this function is called before packageInit

sdk/package/lib/StartSdk.ts

@@ -634,10 +634,7 @@ export class StartSdk<Manifest extends T.SDKManifest> {
        */
       setupInterfaces: setupServiceInterfaces,
       setupMain: (
-        fn: (o: {
+        fn: (o: { effects: Effects }) => Promise<Daemons<Manifest, any>>,
-          effects: Effects
-          started(onTerm: () => PromiseLike<void>): PromiseLike<null>
-        }) => Promise<Daemons<Manifest, any>>,
       ) => setupMain<Manifest>(fn),
       trigger: {
         defaultTrigger,
@@ -690,13 +687,8 @@ export class StartSdk<Manifest extends T.SDKManifest> {
         },
       },
       Daemons: {
-        of(
+        of(effects: Effects) {
-          effects: Effects,
+          return Daemons.of<Manifest>({ effects })
-          started:
-            | ((onTerm: () => PromiseLike<void>) => PromiseLike<null>)
-            | null,
-        ) {
-          return Daemons.of<Manifest>({ effects, started })
         },
       },
       SubContainer: {

sdk/package/lib/mainFn/Daemon.ts

@@ -24,6 +24,7 @@ export class Daemon<
   private commandController: CommandController<Manifest, C> | null = null
   private shouldBeRunning = false
   protected exitedSuccess = false
+  private exiting: Promise<void> | null = null
   private onExitFns: ((success: boolean) => void)[] = []
   protected constructor(
     private subcontainer: C,
@@ -36,7 +37,7 @@ export class Daemon<
     return this.oneshot
   }
   static of<Manifest extends T.SDKManifest>() {
-    return async <C extends SubContainer<Manifest> | null>(
+    return <C extends SubContainer<Manifest> | null>(
       effects: T.Effects,
       subcontainer: C,
       exec: DaemonCommandType<Manifest, C>,
@@ -51,9 +52,7 @@ export class Daemon<
         )
       const res = new Daemon(subc, startCommand)
       effects.onLeaveContext(() => {
-        res
+        res.term({ destroySubcontainer: true }).catch((e) => console.error(e))
-          .term({ destroySubcontainer: true })
-          .catch((e) => console.error(asError(e)))
       })
       return res
     }
@@ -114,19 +113,26 @@ export class Daemon<
     this.shouldBeRunning = false
     this.exitedSuccess = false
     if (this.commandController) {
-      await this.commandController
+      this.exiting = this.commandController.term({ ...termOptions })
-        .term({ ...termOptions })
-        .catch((e) => console.error(asError(e)))
       this.commandController = null
       this.onExitFns = []
+    }
+    if (this.exiting) {
+      await this.exiting.catch(console.error)
       if (termOptions?.destroySubcontainer) {
         await this.subcontainer?.destroy()
       }
+      this.exiting = null
     }
   }
   subcontainerRc(): SubContainerRc<Manifest> | null {
     return this.subcontainer?.rc() ?? null
   }
+  sharesSubcontainerWith(
+    other: Daemon<Manifest, SubContainer<Manifest> | null>,
+  ): boolean {
+    return this.subcontainer?.guid === other.subcontainer?.guid
+  }
   onExit(fn: (success: boolean) => void) {
     this.onExitFns.push(fn)
   }

sdk/package/lib/mainFn/Daemons.ts

@@ -161,9 +161,6 @@ export class Daemons<Manifest extends T.SDKManifest, Ids extends string>
 {
   private constructor(
     readonly effects: T.Effects,
-    readonly started:
-      | ((onTerm: () => PromiseLike<void>) => PromiseLike<null>)
-      | null,
     readonly ids: Ids[],
     readonly healthDaemons: HealthDaemon<Manifest>[],
   ) {}
@@ -180,26 +177,13 @@ export class Daemons<Manifest extends T.SDKManifest, Ids extends string>
    * @param started
    * @returns
    */
-  static of<Manifest extends T.SDKManifest>(options: {
+  static of<Manifest extends T.SDKManifest>(options: { effects: T.Effects }) {
-    effects: T.Effects
+    return new Daemons<Manifest, never>(options.effects, [], [])
-    /**
-     * A closure to run once the system is launched. If you are in main, provide the `started` argument you receive from the function arguments
-     */
-    started: ((onTerm: () => PromiseLike<void>) => PromiseLike<null>) | null
-  }) {
-    return new Daemons<Manifest, never>(
-      options.effects,
-      options.started,
-      [],
-      [],
-    )
   }
 
   private addDaemonImpl<Id extends string>(
     id: Id,
-    daemon: Promise<
+    daemon: Daemon<Manifest, SubContainer<Manifest, T.Effects> | null> | null,
-      Daemon<Manifest, SubContainer<Manifest, T.Effects> | null>
-    > | null,
     requires: Ids[],
     ready: Ready | typeof EXIT_SUCCESS,
   ) {
@@ -215,12 +199,7 @@ export class Daemons<Manifest extends T.SDKManifest, Ids extends string>
     )
     const ids = [...this.ids, id] as (Ids | Id)[]
     const healthDaemons = [...this.healthDaemons, healthDaemon]
-    return new Daemons<Manifest, Ids | Id>(
+    return new Daemons<Manifest, Ids | Id>(this.effects, ids, healthDaemons)
-      this.effects,
-      this.started,
-      ids,
-      healthDaemons,
-    )
   }
 
   /**
@@ -256,7 +235,7 @@ export class Daemons<Manifest extends T.SDKManifest, Ids extends string>
       if (!options) return prev
       const daemon =
         "daemon" in options
-          ? Promise.resolve(options.daemon)
+          ? options.daemon
           : Daemon.of<Manifest>()<C>(
               this.effects,
               options.subcontainer,
@@ -397,12 +376,10 @@ export class Daemons<Manifest extends T.SDKManifest, Ids extends string>
       "EXIT_SUCCESS",
       this.effects,
     )
-    const daemons = await new Daemons<Manifest, Ids>(
+    const daemons = await new Daemons<Manifest, Ids>(this.effects, this.ids, [
-      this.effects,
+      ...this.healthDaemons,
-      this.started,
+      healthDaemon,
-      this.ids,
+    ]).build()
-      [...this.healthDaemons, healthDaemon],
-    ).build()
     try {
       await res
     } finally {
@@ -412,23 +389,51 @@ export class Daemons<Manifest extends T.SDKManifest, Ids extends string>
   }
 
   async term() {
-    for (let result of await Promise.allSettled(
+    const remaining = new Set(this.healthDaemons)
-      this.healthDaemons.map((x) => x.term({ destroySubcontainer: true })),
+
-    )) {
+    while (remaining.size > 0) {
-      if (result.status === "rejected") {
+      // Find daemons with no remaining dependents
-        console.error(result.reason)
+      const canShutdown = [...remaining].filter(
+        (daemon) =>
+          ![...remaining].some((other) =>
+            other.dependencies.some((dep) => dep.id === daemon.id),
+          ),
+      )
+
+      if (canShutdown.length === 0) {
+        // Dependency cycle that should not happen, just shutdown remaining daemons
+        console.warn(
+          "Dependency cycle detected, shutting down remaining daemons",
+        )
+        canShutdown.push(...[...remaining].reverse())
       }
+
+      // remove from remaining set
+      canShutdown.forEach((daemon) => remaining.delete(daemon))
+
+      // Shutdown daemons with no remaining dependents concurrently
+      await Promise.allSettled(
+        canShutdown.map(async (daemon) => {
+          try {
+            console.debug(`Terminating daemon ${daemon.id}`)
+            const destroySubcontainer = daemon.daemon
+              ? ![...remaining].some((d) =>
+                  d.daemon?.sharesSubcontainerWith(daemon.daemon!),
+                )
+              : false
+            await daemon.term({ destroySubcontainer })
+          } catch (e) {
+            console.error(e)
+          }
+        }),
+      )
     }
   }
 
   async build() {
-    this.effects.onLeaveContext(() => {
-      this.term().catch((e) => console.error(asError(e)))
-    })
     for (const daemon of this.healthDaemons) {
       await daemon.init()
     }
-    this.started?.(() => this.term())
     return this
   }
 }

sdk/package/lib/mainFn/HealthDaemon.ts

@@ -34,8 +34,8 @@ export class HealthDaemon<Manifest extends SDKManifest> {
   private resolvedReady: boolean = false
   private readyPromise: Promise<void>
   constructor(
-    private readonly daemon: Promise<Daemon<Manifest>> | null,
+    readonly daemon: Daemon<Manifest> | null,
-    private readonly dependencies: HealthDaemon<Manifest>[],
+    readonly dependencies: HealthDaemon<Manifest>[],
     readonly id: string,
     readonly ready: Ready | typeof EXIT_SUCCESS,
     readonly effects: Effects,
@@ -60,11 +60,9 @@ export class HealthDaemon<Manifest extends SDKManifest> {
     this.running = false
     this.healthCheckCleanup?.()
 
-    await this.daemon?.then((d) =>
+    await this.daemon?.term({
-      d.term({
+      ...termOptions,
-        ...termOptions,
+    })
-      }),
-    )
   }
 
   /** Want to add another notifier that the health might have changed */

sdk/package/lib/mainFn/Oneshot.ts

@@ -15,7 +15,7 @@ export class Oneshot<
   C extends SubContainer<Manifest> | null = SubContainer<Manifest> | null,
 > extends Daemon<Manifest, C> {
   static of<Manifest extends T.SDKManifest>() {
-    return async <C extends SubContainer<Manifest> | null>(
+    return <C extends SubContainer<Manifest> | null>(
       effects: T.Effects,
       subcontainer: C,
       exec: DaemonCommandType<Manifest, C>,

sdk/package/lib/mainFn/index.ts

@@ -15,10 +15,7 @@ export const DEFAULT_SIGTERM_TIMEOUT = 60_000
  * @returns
  */
 export const setupMain = <Manifest extends T.SDKManifest>(
-  fn: (o: {
+  fn: (o: { effects: T.Effects }) => Promise<Daemons<Manifest, any>>,
-    effects: T.Effects
-    started(onTerm: () => PromiseLike<void>): PromiseLike<null>
-  }) => Promise<Daemons<Manifest, any>>,
 ): T.ExpectedExports.main => {
   return async (options) => {
     const result = await fn(options)