world-postgres 4.3.x (latest): close() can resolve while a job is still running — fixed on main, not on the 4.x line
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.4k
- Forks
- 365
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 169
Description
Summary
On the latest release line, PostgresWorld.close() can resolve while a step job is still running, so the application tears down its pool (and exits) underneath a job that has not yet acknowledged completion. The cause is the interaction between Graphile's own signal handlers and runner.stop().
This is already fixed on main. Both fixes are present in the published 5.0.0-beta.44 and absent from 4.3.7, which is what latest currently points at. The ask is a backport to the 4.3.x line, since 5.0 is still beta and moving the whole SDK (workflow@4.8.9 → 5.0.0-beta.53) is a much larger step than the shutdown fix warrants.
The race on 4.3.7
dist/queue.js calls Graphile without noHandleSignals, so Graphile installs its own SIGTERM/SIGINT handlers:
runner = await run({
pgPool: pool,
concurrency: config.queueConcurrency || 10,
logger: graphileLogger,
pollInterval: 500,
taskList,
});
and close() does not await the pool after stopping the runner:
async close() {
closing = true;
// ...
if (runner) {
await runner.stop();
runner = null;
}
if (workerUtils) {
await workerUtils.release();
workerUtils = null;
}
// ...
}
In graphile-worker@0.16.6 (the pinned version), stop() only awaits the worker pool when it is still active:
const promises = [];
if (cron._active) {
promises.push(cron.release());
}
if (workerPool._active) {
promises.push(workerPool.gracefulShutdown());
}
await Promise.all(promises).then(release);
So on SIGTERM the ordering is:
- Graphile's own handler starts
workerPool.gracefulShutdown(), which clears_activebefore in-flight jobs finish. - The application calls
world.close()→runner.stop(). _activeis already false, sostop()awaits nothing, runsrelease()and resolves.close()continues:workerUtils.release(), pool teardown, process exit — while a job still needs a connection to persist its result.
The window is small but real, and it is exactly the case an application-managed shutdown is supposed to cover: the whole point of awaiting world.close() is that it means "jobs are done".
Reproduction
At the Graphile layer, with the same options world-postgres passes. Needs a DATABASE_URL whose role has CREATEDB; it creates and drops a disposable database. On graphile-worker@0.16.6 this prints the failure; with either upstream fix applied it prints PASS.
const { Pool } = require("pg");
const { run } = require("graphile-worker");
const verify = async () => {
const connectionString = process.env.DATABASE_URL;
if (!connectionString) throw new Error("DATABASE_URL with CREATEDB is required");
const databaseName = `gw_shutdown_check_${process.pid}_${Date.now()}`;
const admin = new Pool({ connectionString, connectionTimeoutMillis: 3000 });
const entered = Promise.withResolvers();
const release = Promise.withResolvers();
const originalKill = process.kill;
let taskPool, runner, stopping, created = false, jobFinished = false;
try {
await admin.query(`CREATE DATABASE "${databaseName}"`);
created = true;
const databaseUrl = new URL(connectionString);
databaseUrl.pathname = `/${databaseName}`;
taskPool = new Pool({ connectionString: databaseUrl.toString(), connectionTimeoutMillis: 3000 });
runner = await run({
concurrency: 1,
pgPool: taskPool,
pollInterval: 50,
taskList: {
hold: async () => {
entered.resolve();
await release.promise;
jobFinished = true;
},
},
});
await runner.addJob("hold", {});
await entered.promise;
// Deliver the signal Graphile is listening for, but keep this process alive.
process.kill = (pid, signal) => (pid === process.pid ? true : originalKill(pid, signal));
process.emit("SIGTERM", "SIGTERM");
let stopResolved = false;
stopping = runner.stop().then(() => { stopResolved = true; });
await new Promise((done) => setTimeout(done, 100));
if (stopResolved || jobFinished) {
throw new Error("Runner stopped before its active job finished");
}
release.resolve();
await stopping;
await runner.promise;
const result = await taskPool.query("SELECT count(*)::int AS count FROM graphile_worker.jobs");
if (!jobFinished || result.rows[0].count !== 0) {
throw new Error("Job completion was not acknowledged before stop resolved");
}
console.log("PASS: shutdown waited for the job and its acknowledgement");
} finally {
release.resolve();
if (runner) { await (stopping ?? runner.stop()); await runner.promise; }
await taskPool?.end();
if (created) await admin.query(`DROP DATABASE "${databaseName}"`);
await admin.end();
process.kill = originalKill;
}
};
verify().catch((error) => { console.error(error.message); process.exitCode = 1; });
Expected: the runner stays pending until the job completes and its row is gone.
Actual on 4.3.7's dependency set: Runner stopped before its active job finished.
The fix that already exists on main
5.0.0-beta.44, dist/queue.js:
runner = await run({
pgPool: pool,
concurrency: config.queueConcurrency || 50,
logger: graphileLogger,
...(config.applicationManagedShutdown === true && {
noHandleSignals: true,
}),
pollInterval: 500,
taskList,
});
const activeRunner = runner;
if (activeRunner) {
try {
await activeRunner.stop();
} catch (error) { /* "Runner is already stopped" */ }
await activeRunner.promise.catch(() => {});
runner = null;
}
Either change alone closes the race. Note that 5.0.0-beta.44 still pins graphile-worker@0.16.6, so this is a World-level fix and not something a Graphile bump would deliver.
Request
Backport one or both to 4.3.x. await runner.promise after runner.stop() is the smaller of the two and is safe regardless of who owns the signal, so it would be enough on its own.
If a backport is not planned, it would help to say so — then the answer for self-hosted 4.x users is "pin a patch until 5.0 is stable", which is what we are doing (a three-line patch adding promises.push(workerPool.promise) to Graphile's stop()), and knowing that is the intended state is useful.
Environment
@workflow/world-postgres@4.3.7(latest),workflow@4.8.9,graphile-worker@0.16.6- Next.js 16 App Router, Bun 1.4.2 (
bun --bun next start), self-hosted on Kubernetes - Application-managed shutdown: the HTTP server owns SIGTERM and drains, then awaits
world.close()before exiting
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in dist/queue.js and compare the 4.3.x shutdown path with the corresponding implementation on main. Run the supplied Graphile-layer reproduction with a DATABASE_URL whose role has CREATEDB, then verify that shutdown stays pending until the active job finishes and its row is removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100