Skip to content

Commit 796b197

Browse files
authored
feat: cooperative abort in thread mode + abortGracePeriodMs (#183)
* feat: cooperative abort in thread mode with configurable hard-kill grace Thread-mode jobs can now honor this.abortSignal too. On timeout/cancel the engine transfers a MessagePort to the worker and posts the abort (with its JobTimeout/JobCanceled reason); the worker turns that into the job's abortSignal. A new abortGracePeriodMs config controls how long to wait for the job to stop cooperatively before forcibly terminating the worker thread. abortGracePeriodMs defaults to 0, which preserves the current behavior: the worker is terminated immediately with no cooperative window and no port overhead. The port machinery is only set up when the grace period is positive. Inline is unaffected (no thread to terminate; grace does not apply). * test: cover the worker-side abort port path Adds a test that drives run() through the abortPort branch (thread mode): a real MessageChannel delivers an abort message, signalFromAbortPort rebuilds the JobTimeout reason, and the job observes it via this.abortSignal. Closes the gap where only the engine-side port posting and the inline signal path were tested.
1 parent c78251d commit 796b197

7 files changed

Lines changed: 252 additions & 14 deletions

File tree

packages/core/src/job/abort-reason.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@ export type AbortReason = JobTimeout | JobCanceled;
1010
* Set as the `abortSignal.reason` when a job is aborted because it exceeded its `timeout`.
1111
*/
1212
export class JobTimeout extends Error {
13+
/** The timeout, in milliseconds, that was exceeded. */
14+
readonly timeoutMs: number;
15+
1316
constructor(timeoutMs: number) {
1417
super(`Job timed out after ${timeoutMs}ms`);
1518
this.name = "JobTimeout";
19+
this.timeoutMs = timeoutMs;
1620
}
1721
}
1822

packages/engine/src/engine.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,19 @@ export interface EngineConfig {
6767
* Defaults to `"thread"`.
6868
*/
6969
runner?: "thread" | "inline";
70+
/**
71+
* Grace period, in milliseconds, between cooperatively aborting a running job (timeout or
72+
* cancellation) and forcibly terminating its worker thread.
73+
*
74+
* Only applies to `runner: "thread"`. When greater than `0`, the abort is first delivered to the
75+
* job via `this.abortSignal` so it can stop and clean up; if it has not finished after this many
76+
* milliseconds, the worker thread is terminated. When `0` (default), the worker is terminated
77+
* immediately with no cooperative window, preserving the previous behavior. Has no effect in
78+
* `runner: "inline"` (there is no thread to terminate).
79+
*
80+
* Defaults to `0`.
81+
*/
82+
abortGracePeriodMs?: number;
7083
/** Minimum number of worker threads to use. Defaults to number of CPUs */
7184
minThreads?: number;
7285
/** Maximum number of worker threads to use. Defaults to `minThreads * 2` */
@@ -190,6 +203,7 @@ export class Engine {
190203
gracefulShutdown: config?.gracefulShutdown ?? true,
191204
fork: config?.fork ?? true,
192205
runner: config?.runner ?? "thread",
206+
abortGracePeriodMs: config?.abortGracePeriodMs ?? 0,
193207
minThreads: config?.minThreads ?? cpus().length,
194208
maxThreads: config?.maxThreads ?? cpus().length * 2,
195209
idleWorkerTimeout: config?.idleWorkerTimeout ?? 10_000,

packages/engine/src/shared-runner/runner-pool.test.ts

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { sidequestTest, SidequestTestFixture } from "@/tests/fixture";
2-
import { JobData } from "@sidequest/core";
3-
import EventEmitter from "events";
2+
import { JobData, JobTimeout } from "@sidequest/core";
3+
import { MessagePort } from "node:worker_threads";
44
import { beforeEach, describe, expect, vi } from "vitest";
55
import { DummyJob } from "../test-jobs/dummy-job";
6+
import { AbortPortMessage } from "./runner";
67
import { RunnerPool } from "./runner-pool";
78

89
const piscinaMockInstance = {
@@ -39,14 +40,57 @@ describe("RunnerPool", () => {
3940
pool = new RunnerPool(config);
4041
});
4142

42-
sidequestTest("should call pool.run with job data", async ({ config }) => {
43-
const emiter = new EventEmitter();
44-
const result = await pool.run(jobData, emiter);
43+
sidequestTest("passes the abort signal straight to piscina when grace is 0", async ({ config }) => {
44+
const signal = new AbortController().signal;
45+
const result = await pool.run(jobData, signal);
4546

4647
expect(result).toEqual({ type: "completed", result: "ok" });
47-
expect(piscinaMockInstance.run).toHaveBeenCalledWith({ jobData, config }, { signal: emiter });
48+
expect(piscinaMockInstance.run).toHaveBeenCalledWith({ jobData, config }, { signal });
4849
});
4950

51+
sidequestTest(
52+
"with a grace period, delivers the abort over a port and hard-kills after the grace",
53+
async ({ config }) => {
54+
vi.useFakeTimers();
55+
try {
56+
const gracePool = new RunnerPool({ ...config, abortGracePeriodMs: 1000 });
57+
58+
let capturedPort: MessagePort | undefined;
59+
let hardKillSignal: AbortSignal | undefined;
60+
piscinaMockInstance.run.mockImplementationOnce(
61+
(value: { abortPort: MessagePort }, opts: { signal: AbortSignal }) => {
62+
capturedPort = value.abortPort;
63+
hardKillSignal = opts.signal;
64+
// Resolve only once piscina is asked to terminate (hard kill).
65+
return new Promise((resolve) =>
66+
opts.signal.addEventListener("abort", () => resolve({ type: "completed", result: "killed" })),
67+
);
68+
},
69+
);
70+
71+
const controller = new AbortController();
72+
const runPromise = gracePool.run(jobData, controller.signal);
73+
74+
const message = new Promise<AbortPortMessage>((resolve) =>
75+
capturedPort!.once("message", (m: AbortPortMessage) => resolve(m)),
76+
);
77+
78+
controller.abort(new JobTimeout(5000));
79+
80+
expect(await message).toEqual({ kind: "timeout", timeoutMs: 5000 });
81+
expect(hardKillSignal!.aborted).toBe(false);
82+
83+
// Grace elapses -> piscina is asked to terminate the worker.
84+
await vi.advanceTimersByTimeAsync(1000);
85+
expect(hardKillSignal!.aborted).toBe(true);
86+
87+
await runPromise;
88+
} finally {
89+
vi.useRealTimers();
90+
}
91+
},
92+
);
93+
5094
sidequestTest("should call pool.destroy", () => {
5195
pool.destroy();
5296
expect(piscinaMockInstance.destroy).toHaveBeenCalled();

packages/engine/src/shared-runner/runner-pool.ts

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
import { JobData, JobResult, logger } from "@sidequest/core";
1+
import { JobData, JobResult, JobTimeout, logger } from "@sidequest/core";
2+
import { MessageChannel } from "node:worker_threads";
23
import Piscina from "piscina";
34
import { DEFAULT_RUNNER_PATH } from "../constants";
45
import { NonNullableEngineConfig } from "../engine";
56
import { JobRunner } from "./job-runner";
7+
import type { AbortPortMessage } from "./runner";
68

79
/**
810
* A pool of worker threads for running jobs in parallel using Piscina.
@@ -29,13 +31,54 @@ export class RunnerPool implements JobRunner {
2931

3032
/**
3133
* Runs a job in the worker pool.
34+
*
35+
* With `abortGracePeriodMs === 0` (default), an abort terminates the worker immediately. With a
36+
* positive grace period, the abort is delivered to the job cooperatively over a transferred port
37+
* (so it can stop via `this.abortSignal`), and the worker is only forcibly terminated if it has
38+
* not finished within the grace period.
39+
*
3240
* @param job The job data to run.
33-
* @param signal Optional abort signal; when it aborts, piscina terminates the worker thread.
41+
* @param signal Optional abort signal for cancellation/timeout.
3442
* @returns A promise resolving to the job result.
3543
*/
3644
run(job: JobData, signal?: AbortSignal): Promise<JobResult> {
3745
logger("RunnerPool").debug(`Running job ${job.id} in pool`);
38-
return this.pool.run({ jobData: job, config: this.nonNullConfig }, { signal });
46+
const grace = this.nonNullConfig.abortGracePeriodMs;
47+
48+
if (!signal || grace <= 0) {
49+
// Abort terminates the worker immediately.
50+
return this.pool.run({ jobData: job, config: this.nonNullConfig }, { signal });
51+
}
52+
53+
// Deliver the abort cooperatively first, then hard-terminate after the grace period.
54+
const channel = new MessageChannel();
55+
const hardKill = new AbortController();
56+
let graceTimer: ReturnType<typeof setTimeout> | undefined;
57+
58+
const onAbort = () => {
59+
const reason: unknown = signal.reason;
60+
const message: AbortPortMessage =
61+
reason instanceof JobTimeout ? { kind: "timeout", timeoutMs: reason.timeoutMs } : { kind: "canceled" };
62+
channel.port1.postMessage(message);
63+
graceTimer = setTimeout(() => hardKill.abort(), grace);
64+
};
65+
66+
if (signal.aborted) {
67+
onAbort();
68+
} else {
69+
signal.addEventListener("abort", onAbort, { once: true });
70+
}
71+
72+
return this.pool
73+
.run(
74+
{ jobData: job, config: this.nonNullConfig, abortPort: channel.port2 },
75+
{ transferList: [channel.port2], signal: hardKill.signal },
76+
)
77+
.finally(() => {
78+
if (graceTimer) clearTimeout(graceTimer);
79+
signal.removeEventListener("abort", onAbort);
80+
channel.port1.close();
81+
});
3982
}
4083

4184
/**

packages/engine/src/shared-runner/runner.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { sidequestTest, SidequestTestFixture } from "@/tests/fixture";
2-
import { FailedResult, JobData } from "@sidequest/core";
2+
import { FailedResult, JobCanceled, JobData, JobTimeout } from "@sidequest/core";
33
import { existsSync, unlinkSync } from "node:fs";
44
import { unlink, writeFile } from "node:fs/promises";
55
import { join, resolve } from "node:path";
6+
import { MessageChannel } from "node:worker_threads";
67
import { vi } from "vitest";
8+
import { AbortAwareJob } from "../test-jobs/abort-aware-job";
79
import { DummyJob } from "../test-jobs/dummy-job";
810
import { DummyJobWithArgs } from "../test-jobs/dummy-job-with-args";
911
import { importSidequest } from "../utils/import";
@@ -80,6 +82,80 @@ describe("runner.ts", () => {
8082
injectSpy.mockRestore();
8183
});
8284

85+
sidequestTest("a job receives the abort signal and its reason and stops", async ({ backend, config }) => {
86+
const job = new AbortAwareJob();
87+
await job.ready();
88+
const abortJobData = await backend.createNewJob({
89+
class: job.className,
90+
script: job.script,
91+
args: [],
92+
attempt: 0,
93+
queue: "default",
94+
constructor_args: [],
95+
state: "waiting",
96+
});
97+
98+
const controller = new AbortController();
99+
const resultPromise = run({ jobData: abortJobData, config, inline: true, signal: controller.signal });
100+
controller.abort(new JobCanceled());
101+
102+
// The job resolves once it observes the abort (whether before it starts or while awaiting it).
103+
const result = (await resultPromise) as FailedResult;
104+
expect(result.type).toBe("failed");
105+
expect(result.error.message).toContain("JobCanceled");
106+
});
107+
108+
sidequestTest("a job sees an already-aborted signal before it starts", async ({ backend, config }) => {
109+
const job = new AbortAwareJob();
110+
await job.ready();
111+
const abortJobData = await backend.createNewJob({
112+
class: job.className,
113+
script: job.script,
114+
args: [],
115+
attempt: 0,
116+
queue: "default",
117+
constructor_args: [],
118+
state: "waiting",
119+
});
120+
121+
const controller = new AbortController();
122+
controller.abort(new JobTimeout(50));
123+
124+
const result = (await run({
125+
jobData: abortJobData,
126+
config,
127+
inline: true,
128+
signal: controller.signal,
129+
})) as FailedResult;
130+
expect(result.type).toBe("failed");
131+
expect(result.error.message).toContain("aborted before start: JobTimeout");
132+
});
133+
134+
sidequestTest("a thread job is aborted via the abort port (rebuilds the reason)", async ({ backend, config }) => {
135+
const job = new AbortAwareJob();
136+
await job.ready();
137+
const abortJobData = await backend.createNewJob({
138+
class: job.className,
139+
script: job.script,
140+
args: [],
141+
attempt: 0,
142+
queue: "default",
143+
constructor_args: [],
144+
state: "waiting",
145+
});
146+
147+
const channel = new MessageChannel();
148+
// Thread path: no live signal, the abort arrives over the transferred port.
149+
const resultPromise = run({ jobData: abortJobData, config, abortPort: channel.port2 });
150+
channel.port1.postMessage({ kind: "timeout", timeoutMs: 1234 });
151+
152+
const result = (await resultPromise) as FailedResult;
153+
expect(result.type).toBe("failed");
154+
// The worker reconstructs the JobTimeout reason from the port message.
155+
expect(result.error.message).toContain("JobTimeout");
156+
channel.port1.close();
157+
});
158+
83159
sidequestTest("fails with invalid script", async ({ config }) => {
84160
jobData.script = "invalid!";
85161
const result = (await run({ jobData, config })) as FailedResult;

packages/engine/src/shared-runner/runner.ts

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,40 @@
1-
import { Job, JobClassType, JobData, JobResult, logger, resolveScriptPathForJob, toErrorData } from "@sidequest/core";
1+
import {
2+
Job,
3+
JobCanceled,
4+
JobClassType,
5+
JobData,
6+
JobResult,
7+
JobTimeout,
8+
logger,
9+
resolveScriptPathForJob,
10+
toErrorData,
11+
} from "@sidequest/core";
212
import { existsSync } from "node:fs";
313
import { fileURLToPath } from "node:url";
14+
import { MessagePort } from "node:worker_threads";
415
import { EngineConfig } from "../engine";
516
import { importSidequest } from "../utils";
617
import { findSidequestJobsScriptInParentDirs, MANUAL_SCRIPT_TAG, resolveScriptPath } from "./manual-loader";
718

19+
/** Message posted by the engine over the abort port to cooperatively abort a worker-thread job. */
20+
export type AbortPortMessage = { kind: "timeout"; timeoutMs: number } | { kind: "canceled" };
21+
22+
/**
23+
* Builds an {@link AbortSignal} for a worker-thread job from an abort port.
24+
*
25+
* The thread runner cannot receive a live `AbortSignal` across the worker boundary, so the engine
26+
* transfers a {@link MessagePort} and posts an abort message on it; this turns that message into a
27+
* local signal carrying the proper {@link JobTimeout}/{@link JobCanceled} reason.
28+
*/
29+
function signalFromAbortPort(port: MessagePort): AbortSignal {
30+
const controller = new AbortController();
31+
port.on("message", (message: AbortPortMessage) => {
32+
const reason = message.kind === "timeout" ? new JobTimeout(message.timeoutMs) : new JobCanceled();
33+
controller.abort(reason);
34+
});
35+
return controller.signal;
36+
}
37+
838
/**
939
* Runs a job by dynamically importing its script and executing the specified class.
1040
* @param jobData The job data containing script and class information
@@ -16,11 +46,13 @@ export default async function run({
1646
config,
1747
inline,
1848
signal,
49+
abortPort,
1950
}: {
2051
jobData: JobData;
2152
config: EngineConfig;
2253
inline?: boolean;
2354
signal?: AbortSignal;
55+
abortPort?: MessagePort;
2456
}): Promise<JobResult> {
2557
// In inline mode the job runs in the host process, where Sidequest is already configured, so
2658
// re-injecting the config is redundant. In a worker thread the module is fresh and needs it.
@@ -79,12 +111,18 @@ export default async function run({
79111

80112
const job: Job = new JobClass(...jobData.constructor_args);
81113
job.injectJobData(jobData);
82-
if (signal) {
83-
job.injectAbortSignal(signal);
114+
// Inline passes a live signal directly; the thread runner gets an abort port it turns into one.
115+
const abortSignal = signal ?? (abortPort ? signalFromAbortPort(abortPort) : undefined);
116+
if (abortSignal) {
117+
job.injectAbortSignal(abortSignal);
84118
}
85119

86120
logger("Runner").debug(`Executing job class "${jobData.class}" with args:`, jobData.args);
87-
return job.perform(...jobData.args);
121+
try {
122+
return await job.perform(...jobData.args);
123+
} finally {
124+
abortPort?.close();
125+
}
88126
}
89127

90128
/**
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { Job } from "@sidequest/core";
2+
3+
/**
4+
* A job that honors `this.abortSignal`: it waits until aborted and reports the abort reason.
5+
* Used to verify cooperative cancellation/timeout end-to-end through the runner.
6+
*/
7+
export class AbortAwareJob extends Job {
8+
async run() {
9+
if (this.abortSignal.aborted) {
10+
return this.fail(`aborted before start: ${this.abortSignal.reason?.name}`);
11+
}
12+
13+
await new Promise((resolve) => {
14+
this.abortSignal.addEventListener("abort", resolve, { once: true });
15+
});
16+
17+
return this.fail(`aborted: ${this.abortSignal.reason?.name}`);
18+
}
19+
}

0 commit comments

Comments
 (0)