Skip to content

Commit 9bb8eb8

Browse files
fix(desktop): let lifecycle shutdown finish before exiting
On quit, the generic Effect runtime (NodeRuntime.runMain) installs its own SIGINT/SIGTERM handler that interrupts the main fiber the moment a signal arrives — including while DesktopLifecycle is still waiting for the backend to stop its provider sessions. The backend is cut off mid-shutdown and in-flight provider work is abandoned. DesktopRuntime.runMain replaces it: built with Runtime.makeRunMain, it drives teardown on fiber exit but installs no competing signal handler, so DesktopLifecycle alone owns the signals and its graceful shutdown runs to completion before Electron exits. The backend terminate grace moves 5s -> 30s. Both callers that stop backend instances — the quit path and the update-install path, whose quitAndInstall bypasses before-quit cleanup — now wait for one BACKEND_STOP_TIMEOUT derived from that grace plus the output drain, instead of a hardcoded 5s each. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 67c0fc0 commit 9bb8eb8

7 files changed

Lines changed: 98 additions & 10 deletions

File tree

‎apps/desktop/src/app/DesktopApp.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as Cause from "effect/Cause";
2-
import * as Duration from "effect/Duration";
32
import * as Effect from "effect/Effect";
43
import * as Option from "effect/Option";
54
import * as Ref from "effect/Ref";
@@ -18,6 +17,7 @@ import * as DesktopClerk from "./DesktopClerk.ts";
1817
import * as DesktopApplicationMenu from "../window/DesktopApplicationMenu.ts";
1918
import * as DesktopWindow from "../window/DesktopWindow.ts";
2019
import * as PreviewPasskeys from "../preview/Passkeys.ts";
20+
import * as DesktopBackendManager from "../backend/DesktopBackendManager.ts";
2121
import * as DesktopBackendPool from "../backend/DesktopBackendPool.ts";
2222
import * as DesktopEnvironment from "./DesktopEnvironment.ts";
2323
import * as DesktopLegacyLocalStorage from "./DesktopLegacyLocalStorage.ts";
@@ -153,7 +153,7 @@ export const stopAllPoolInstances = Effect.fn("desktop.app.stopAllPoolInstances"
153153
const instances = yield* pool.list;
154154
yield* Effect.forEach(
155155
instances,
156-
(instance) => instance.stop({ timeout: Duration.seconds(5) }),
156+
(instance) => instance.stop({ timeout: DesktopBackendManager.BACKEND_STOP_TIMEOUT }),
157157
{ concurrency: "unbounded" },
158158
);
159159
},
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// @effect-diagnostics-next-line nodeBuiltinImport:off - A real Node process verifies OS signal ownership outside Effect's process runtime.
2+
import * as NodeChildProcess from "node:child_process";
3+
import * as NodeReadline from "node:readline";
4+
import * as NodeURL from "node:url";
5+
import { expect, it } from "@effect/vitest";
6+
7+
it("leaves SIGTERM to the lifecycle until its graceful shutdown completes", async ({
8+
onTestFinished,
9+
}) => {
10+
const runner = new URL("./DesktopRuntime.ts", import.meta.url).href;
11+
const child = NodeChildProcess.spawn(
12+
process.execPath,
13+
[
14+
"--experimental-strip-types",
15+
"--input-type=module",
16+
"-e",
17+
`
18+
import * as Effect from 'effect/Effect';
19+
import * as Deferred from 'effect/Deferred';
20+
import { runMain } from ${JSON.stringify(runner)};
21+
let released = false;
22+
runMain(Effect.scoped(Effect.gen(function* () {
23+
const done = yield* Deferred.make();
24+
yield* Effect.addFinalizer(() => Effect.sync(() => console.log(released ? 'CLEANED' : 'PREMATURE')));
25+
process.once('SIGTERM', () => {
26+
console.log('STOPPING');
27+
process.stdin.once('data', () => {
28+
released = true;
29+
void Effect.runPromise(Deferred.succeed(done, undefined)).then(() => process.stdin.destroy());
30+
});
31+
});
32+
console.log('READY');
33+
yield* Deferred.await(done);
34+
})));
35+
`,
36+
],
37+
{
38+
cwd: NodeURL.fileURLToPath(new URL("../../", import.meta.url)),
39+
stdio: ["pipe", "pipe", "pipe"],
40+
},
41+
);
42+
const exited = new Promise<readonly [number | null, string | null]>((resolve, reject) => {
43+
child.once("exit", (code, signal) => resolve([code, signal]));
44+
child.once("error", reject);
45+
});
46+
const lines: string[] = [];
47+
let stderr = "";
48+
child.stderr.on("data", (chunk) => {
49+
stderr += chunk;
50+
});
51+
onTestFinished(async () => {
52+
if (child.exitCode === null && child.signalCode === null) {
53+
child.kill("SIGKILL");
54+
await exited;
55+
}
56+
});
57+
for await (const line of NodeReadline.createInterface({ input: child.stdout })) {
58+
lines.push(line);
59+
if (line === "READY") child.kill("SIGTERM");
60+
if (line === "STOPPING") child.stdin.write("finish");
61+
}
62+
expect(await exited, stderr).toEqual([0, null]);
63+
expect(lines).toEqual(["READY", "STOPPING", "CLEANED"]);
64+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import * as Runtime from "effect/Runtime";
2+
3+
// DesktopLifecycle owns signals and waits for backend shutdown before quitting
4+
// Electron. NodeRuntime's competing signal handler interrupts that sequence.
5+
export const runMain = Runtime.makeRunMain(({ fiber, teardown }) => {
6+
fiber.addObserver((exit) => {
7+
teardown(exit, (code) => {
8+
if (code !== 0) process.exit(code);
9+
});
10+
});
11+
});

‎apps/desktop/src/backend/DesktopBackendManager.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ describe("DesktopBackendManager", () => {
262262
assert.equal(spawnedCommand.options.additionalFds?.fd5?.type, "output");
263263
assert.equal(
264264
Duration.toMillis(Duration.fromInputUnsafe(spawnedCommand.options.forceKillAfter)),
265-
5_000,
265+
30_000,
266266
);
267267

268268
assert.deepEqual(yield* decodeBootstrap(bootstrapJson), configWithObservability);
@@ -1596,7 +1596,10 @@ describe("DesktopBackendManager", () => {
15961596
const started = yield* Queue.takeN(teardownStarted, 2);
15971597
assert.deepEqual(started.toSorted(), ["instance1", "instance2"]);
15981598

1599-
yield* TestClock.adjust(Duration.seconds(5));
1599+
yield* TestClock.adjust(Duration.seconds(6));
1600+
assert.isUndefined(quitFiber.pollUnsafe());
1601+
yield* TestClock.adjust(Duration.seconds(39));
1602+
16001603
yield* Fiber.join(quitFiber);
16011604
assert.equal(yield* Queue.size(teardownFinished), 0);
16021605

‎apps/desktop/src/backend/DesktopBackendManager.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,19 @@ const MAX_PREFLIGHT_FAILURE_ATTEMPTS = 5;
3737
const DEFAULT_BACKEND_READINESS_TIMEOUT = Duration.minutes(1);
3838
const DEFAULT_BACKEND_READINESS_INTERVAL = Duration.millis(100);
3939
const DEFAULT_BACKEND_READINESS_REQUEST_TIMEOUT = Duration.seconds(1);
40-
// Same as the service launcher. A killed backend leaves its agents running,
41-
// and a busy one needs a few seconds to stop every provider session.
42-
const DEFAULT_BACKEND_TERMINATE_GRACE = Duration.seconds(5);
40+
// A killed backend leaves its agents running, and a busy one needs time to
41+
// stop every provider session, so an interactive quit waits for that instead
42+
// of racing it. Longer than the service launcher's 5s because the desktop
43+
// quit/update path should not abandon in-flight provider work.
44+
const DEFAULT_BACKEND_TERMINATE_GRACE = Duration.seconds(30);
4345
const DEFAULT_BACKEND_OUTPUT_DRAIN_TIMEOUT = Duration.seconds(5);
46+
// How long a caller of `stop` waits for an instance. It must outlast the
47+
// termination grace plus output drain, or quit and update give up while the
48+
// backend is still stopping its agents.
49+
export const BACKEND_STOP_TIMEOUT = Duration.sum(
50+
Duration.sum(DEFAULT_BACKEND_TERMINATE_GRACE, DEFAULT_BACKEND_OUTPUT_DRAIN_TIMEOUT),
51+
Duration.seconds(10),
52+
);
4453
const BACKEND_READINESS_PATH = "/.well-known/t3/environment";
4554
const { logWarning: logBackendProcessWarning } =
4655
DesktopObservability.makeComponentLogger("desktop-backend-process");

‎apps/desktop/src/main.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ for (const stream of [process.stdout, process.stderr]) {
66
}
77

88
import * as NodeHttpClient from "@effect/platform-node/NodeHttpClient";
9-
import * as NodeRuntime from "@effect/platform-node/NodeRuntime";
9+
import * as DesktopRuntime from "./app/DesktopRuntime.ts";
1010
import * as NodeServices from "@effect/platform-node/NodeServices";
1111
// @effect-diagnostics-next-line nodeBuiltinImport:off - Version output must flush before Electron exits, without acquiring the runtime.
1212
import * as NodeFS from "node:fs";
@@ -242,4 +242,4 @@ const layerDesktopRuntime = layerDesktopClerk.pipe(
242242
Layer.provideMerge(DesktopPreReadyPlatform.layer),
243243
);
244244

245-
DesktopApp.program.pipe(Effect.provide(layerDesktopRuntime), NodeRuntime.runMain);
245+
DesktopApp.program.pipe(Effect.provide(layerDesktopRuntime), DesktopRuntime.runMain);

‎apps/desktop/src/updates/DesktopUpdates.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import * as Scope from "effect/Scope";
2424
import * as Semaphore from "effect/Semaphore";
2525
import * as Stream from "effect/Stream";
2626

27+
import * as DesktopBackendManager from "../backend/DesktopBackendManager.ts";
2728
import * as DesktopBackendPool from "../backend/DesktopBackendPool.ts";
2829
import * as DesktopConfig from "../app/DesktopConfig.ts";
2930
import * as DesktopEnvironment from "../app/DesktopEnvironment.ts";
@@ -755,7 +756,7 @@ export const make = Effect.gen(function* () {
755756
yield* Ref.set(installRunningBackendsRef, runningInstances);
756757
yield* Effect.forEach(
757758
instances,
758-
(instance) => instance.stop({ timeout: Duration.seconds(5) }),
759+
(instance) => instance.stop({ timeout: DesktopBackendManager.BACKEND_STOP_TIMEOUT }),
759760
{ concurrency: "unbounded" },
760761
);
761762
yield* electronUpdater.quitAndInstall({

0 commit comments

Comments
 (0)