Skip to content

Commit df6f7ca

Browse files
bluestreak01claude
andcommitted
Fix dropped job when close races a busy worker
QueryWorker.runLoop checked `while (!shuttingDown)` at the top of the loop. After runOn() returned, control hit that check without taking signalLock or re-reading `current`, so QuestDB.close() shutting down a borrowed, busy worker could exit the loop with a job already pending: 1. The worker is inside runOn(q1); the terminal callback fires inline, signals done, and wakes the app thread. 2. The app thread's await() returns and it submits again on the same reused lease. dispatch() reads shuttingDown==false, sets current=q2. 3. QuestDB.close() -> queryPool.close() -> worker.shutdown() flips shuttingDown to true (close() snapshots every worker in `all`, borrowed ones included). 4. The worker returns from runOn(q1), re-checks the loop top, sees shuttingDown==true, and exits -- without re-inspecting current. q2 is then never run, never stranded, and never signalled, so the app thread's next await() blocks forever. dispatch()'s own shuttingDown check does not catch it (dispatch won the race and read false), the shutdown interrupt never reaches it (q2 was never handed to the client), and client.close()'s synthetic terminal cannot reach it either. The strand logic that signals a pending current already lives inside the signalLock block (the `if (shuttingDown) return` branch). Looping unconditionally routes every shutdown ordering through that single strand point, and the loop still terminates via the same return. The existing QueryWorkerTest.testShutdownRacingDispatchMustNotStrandCaller only drove the parked-worker variant, so this busy-worker path was untested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7948635 commit df6f7ca

1 file changed

Lines changed: 12 additions & 1 deletion

File tree

core/src/main/java/io/questdb/client/impl/QueryWorker.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,18 @@ void dispatch(QueryImpl q) {
269269
}
270270

271271
private void runLoop() {
272-
while (!shuttingDown) {
272+
// Loop unconditionally -- do NOT hoist the shuttingDown check up here as
273+
// while (!shuttingDown). The sole exit is the "if (shuttingDown) return"
274+
// inside the signalLock block below, which strands a pending current
275+
// before returning. Exiting at the top instead would skip that strand on
276+
// the busy-worker path: when a reused lease's submit() -> dispatch() sets
277+
// current between the terminal callback and this check, and shutdown()
278+
// then flips shuttingDown, the worker would return straight after
279+
// runOn() without re-inspecting current -- the job is dropped, never
280+
// run, never signalled, and its caller's await() hangs forever.
281+
// Re-entering the lock every lap funnels every shutdown ordering through
282+
// the single strand point.
283+
while (true) {
273284
QueryImpl q;
274285
signalLock.lock();
275286
try {

0 commit comments

Comments
 (0)