processCallback / never cleaned up #252

Activity

1. danielebriggi

   assigned

   danielebriggi

   and

   andinux

   on Sep 5, 2025
2. danielebriggi

   added the

   Bug

   issue type on Sep 5, 2025
3. andinux commented on Aug 19, 2026

   

   andinux

   on Aug 19, 2026

   Collaborator

   More actions

   Ran into the mirror image of the first bullet while stress-testing the core server, and the two look like the same root cause. Line numbers below are against 614e739.

   The callback can also be invoked zero times. The first bullet covers processCallback firing again for a previous command; there is a second direction where it never fires at all, and the caller's promise simply never settles.

   close() (src/drivers/connection-tls.ts:276) tears everything down without settling anything:

   close(): this {
     if (this.socket) {
       this.socket.removeAllListeners()
       this.socket.destroy()
       this.socket = undefined
     }
     this.operations.clear()
     return this
   }

   No processCommandsFinish, so a command that was in flight when the caller closed the connection is dropped silently.

   OperationsQueue.clear() (src/drivers/queue.ts:21) is the second half:

   public clear(): void { this.queue = []; this.isProcessing = false }

   sendCommands (src/drivers/connection.ts:88) wraps the user callback inside the queued operation, so discarding the operation discards the callback with it — no done, no error, nothing.

   And the socket handlers (connection-tls.ts:112, :117, :122, plus 'timeout') all call this.close() before processCommandsFinish, so the queue is cleared first: the command currently transporting still gets its callback, but anything queued behind it is gone.

   One case that belongs to the first bullet as written. connect() sends initializationCommands through transportCommands directly (connection-tls.ts:96), bypassing the operations queue. A sendCommands issued before initialization completes overwrites this.processCallback (:90 / :150) while the init response is still outstanding — so the new command's callback can receive the init response. That is exactly "the callback used in the previous command", just triggered by ordering rather than by a socket error. I have not confirmed this one firing in practice.

   processCallback is still never cleared on 614e739: the only assignments are at :90 and :150, both setting a new callback, and nothing ever sets it back to undefined.

   Why it matters in practice. A test suite that deliberately drops connections mid-command hangs until the test framework's timeout rather than the driver's, which reads like a server hang and is not one — the server answered fresh queries the whole time. It cost me a while to work out that the stall was client-side. Anything doing failover or cancellation on top of the driver would see the same.

   Suggestion, matching the second bullet. Both directions go away with one invariant: every accepted command settles exactly once. Concretely, settle-and-clear processCallback inside processCommandsFinish, and have close() (and OperationsQueue.clear()) settle whatever is outstanding with an error instead of discarding it. Worth fixing both directions together, otherwise fixing only the double-call half leaves the never-called half in place.