diff --git a/apps/server/src/cloud/selfUpdate.test.ts b/apps/server/src/cloud/selfUpdate.test.ts index 655228c047f..bfac916a59d 100644 --- a/apps/server/src/cloud/selfUpdate.test.ts +++ b/apps/server/src/cloud/selfUpdate.test.ts @@ -545,11 +545,19 @@ it.layer(NodeServices.layer)("ServerSelfUpdate.update", (it) => { yield* TestClock.adjust(Duration.seconds(10)); assert.deepEqual(context.commands[3], { command: "systemctl", - args: ["--user", "restart", "t3code.service"], + args: ["--user", "restart", "--no-block", "t3code.service"], }); assert.lengthOf(context.spawns, 0); // systemd replaces the process; the server must not exit itself. assert.equal(context.exitCount(), 0); + + // The queued restart returns while this process is still shutting + // down; the lock must stay held so a second update cannot rewrite the + // unit mid-teardown. + const concurrentError = yield* context.service + .update({ targetVersion: "0.0.30" }) + .pipe(Effect.flip); + assert.include(concurrentError.reason, "already in progress"); }).pipe(Effect.provide(TestClock.layer())), ); @@ -583,7 +591,7 @@ it.layer(NodeServices.layer)("ServerSelfUpdate.update", (it) => { assert.deepEqual( context.commands.slice(-2).map((entry) => entry.args), [ - ["--user", "restart", BOOT_SERVICE_UNIT_FILE], + ["--user", "restart", "--no-block", BOOT_SERVICE_UNIT_FILE], ["--user", "daemon-reload"], ], ); diff --git a/apps/server/src/cloud/selfUpdate.ts b/apps/server/src/cloud/selfUpdate.ts index f786cbea8cf..9dcb713e1a1 100644 --- a/apps/server/src/cloud/selfUpdate.ts +++ b/apps/server/src/cloud/selfUpdate.ts @@ -354,14 +354,19 @@ export const make = Effect.fn("cloud.server_self_update.make")(function* (option targetVersion, }); // Restart after the acknowledgement has had time to cross any relay - // hop. If systemd rejects the handoff, restore the previous unit while - // this process is still alive and log the failure for diagnostics. + // hop. --no-block queues the restart job and exits before systemd + // stops this unit: a blocking restart's SIGTERM reaches the systemctl + // child (it shares this service's cgroup), which read as a restart + // failure and rolled the new unit back while the old server finished + // shutting down. With the handoff race gone, a non-zero exit or spawn + // error means systemd genuinely rejected the job while this process is + // still alive, so restoring the previous unit below stays correct. yield* scheduleRestart( Effect.gen(function* () { const restart = yield* runner .run({ command: "systemctl", - args: ["--user", "restart", BOOT_SERVICE_UNIT_FILE], + args: ["--user", "restart", "--no-block", BOOT_SERVICE_UNIT_FILE], }) .pipe( Effect.mapError((cause) => @@ -389,9 +394,13 @@ export const make = Effect.fn("cloud.server_self_update.make")(function* (option Effect.catch((error) => Effect.logError("Server self-update could not restart the boot service.").pipe( Effect.annotateLogs({ targetVersion, error: error.reason }), + // Permit a retry only after the failed handoff was rolled + // back. A queued restart returns while this process is still + // shutting down; releasing the lock then would let a second + // update rewrite the unit mid-teardown. + Effect.andThen(Ref.set(inFlight, false)), ), ), - Effect.ensuring(Ref.set(inFlight, false)), ), ); } else {