[dhensby]

What goes wrong

A stream paused before its query reaches a result set has msnodesqlv8's query held paused. toReadableStream() leaves a stream like this until it is read, and so does request.pause() before query(). msnodesqlv8 still runs the statements ahead of the first result set, because begin() and nextResult() don't check paused. It cancels a paused query by freeing its statement.

That free can land after the server has sent the row count of one of those statements, while msnodesqlv8 is in nextResult (SQLMoreResults). The process then crashes with a segfault. An example query:

set nocount off; insert ...; raiserror('flush', 0, 1) with nowait; waitfor delay '00:00:02'; select ...

Any batch or procedure that runs DML with NOCOUNT OFF and gets its output flushed before its first result set can do this.

This bug is already on master and is independent of #1928. Copilot's review of #1928 led to it, because #1928 sends a stream resumed while its query executes down the same path.

The fix

A query held back before its first result set is now cancelled once it stops at that result set, where it has nothing under way:

• It reaches a result set: the cancel is made there, through the existing cancelDeferred path, which already does this for a paused stream part way through a result set.
• It ends first: it completes as cancelled.
• msnodesqlv8 has sent the query its one cancel already (5.x, from a cancel token or its own timeout on Linux with ODBC Driver 17): that cancel was made while the query was paused, so it goes through, and the request completes as cancelled. Before this, the deferred cancel was dropped and a paused stream never completed.

The cost: such a cancel no longer cuts short the statements ahead of the first result set. It takes effect when they end, or when the request times out, and requestTimeout: 0 (or under a second) means nothing limits the wait. Errors from those statements, including a timeout, are streamed ahead of the ECANCEL. The README's cancel section now says so.

No adapter-side signal could do better. Nothing tells the request when msnodesqlv8 moves from SQLExecDirect into nextResult, and a cancel during SQLExecDirect itself is safe.

Live results (Windows CI, SQL Server 2022)

These ran on a throwaway fork branch, since deleted.

msnodesqlv8	new integration test, master's adapter (6 runs over two rounds)	new integration test, this PR (3 runs)	full msnodesqlv8 suite, this PR
^2 (2.7)	5 crashed	3 passed	161 passing
^3 (3.1)	6 crashed	3 passed	158 passing
^4 (4.5)	6 crashed	3 passed	158 passing
5.2.1	6 crashed	3 passed	158 passing
^5 (5.6.0)	6 crashed	3 passed	158 passing

With this PR, the stream ends with ECANCEL at the query's first result set, about 2 s in, and the next query on the connection runs.

Earlier probes of master's adapter narrowed down what triggers the crash. Each of these ran 3 times per version, across all 5 versions:

Scenario	Result on master's adapter
Cancel during the first statement's execute (waitfor; select)	no crash, prompt ECANCEL
Row count without a flush	no crash
Flush without a row count	no crash
Held with no cancel	no crash
Row count + flush, then cancel	crashed in 14 of 15 runs

The same row count + flush cancel, deferred to the first result set as this PR does it, gave no crashes in any of 120 runs.

The crash reproduces with msnodesqlv8 on its own, too. Over two rounds of three runs per ODBC driver, it crashed in all 12 runs on 5.2.1 and in 11 of 12 on 5.6.0, the latest release, with both ODBC Driver 17 and Native Client 11. I'll report it upstream.

Tests

Unit tests (test/common/unit.js):

• A held query is not cancelled while it runs, nor while it moves on past a row count and a flushed message. It is cancelled at its first result set. The same holds for a held query cancelled before it starts.
• A held stream resumed after its cancel stays paused until the cancel is made.
• A held query cancelled after an error that may end the batch is cancelled at its first result set.
• A held query that ends without a result set completes as cancelled. One that times out first streams the timeout, then the cancel.
• If msnodesqlv8 has already sent its one cancel by the first result set, the request still completes as cancelled, without sending a second.
• The simulator's stallBegin test, which cancelled a held query during its first statement, now expects the cancel when that statement ends.

Master's adapter fails 7 of these. Each mutant of the fix fails at least one test, apart from two that are equivalent on master.

Integration test: cancelling a stream held back while its query moves on to its first result set runs against both drivers on a pool of one connection. It cancels a held stream 500 ms into the repro query, expects ECANCEL and no rows, and then checks that the next query runs.

Local checks

• npm run lint: clean.
• npm run test-unit: 256 passing.
• test-tedious against Azure SQL Edge: 179 passing. The 3 failures are the CLR-dependent tests that always fail on SQL Edge.

Relation to #1928

#1928 (resume during execute, #1924) is rebased on this once it merges. With this in place, a stream resumed while its query executes and then cancelled takes the same deferred path. It no longer hits the crash.

msnodesqlv8 6

msnodesqlv8 6.0.0 fixes this crash itself. It also emits nothing for a paused query until it is resumed, meta and done included, so the deferred cancel would wait forever there. The deferral is therefore switched off from msnodesqlv8 6, read from the major version in its package.json, and the cancel is made at once, as on master.

• Unit tests cover 6.0.0 (cancelled at once), 5.6.0 (deferred) and a version that can't be read (deferred).
• A separate ci: commit runs the msnodesqlv8 suite against v6 on Node 22 and 24 in the Windows jobs. msnodesqlv8 6 needs Node 22 or later.
• The README's cancel section now limits the wait to msnodesqlv8 before 6.

The rest of the v6 work is tracked in #1940.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

Version-specific native-driver cancellation ordering warrants human review, and the integration test’s timeout cleanup still needs correction.

1 open issue

• Mark connection held before cancelled-stream timeout cleanup · New

What changed in this PR

This PR prevents native-driver crashes when cancelling an msnodesqlv8 stream paused before its first result set, integrating the workaround into the existing deferred-cancellation handling.

Changes:

• Defer cancellation until the first result set or query completion, preserving cancellation when the driver has already sent it.
• Add unit and cross-driver integration coverage for cancellation and connection reuse.
• Document delayed cancellation, retained statement effects, and timeout limits.

File	Description
test/​tedious/​tedious.js	Registers the shared cancellation regression test.
test/​msnodesqlv8/​msnodesqlv8.js	Registers the shared cancellation regression test.
test/​common/​unit.js	Covers deferred cancellation and driver-event edge cases.
test/​common/​tests.js	Tests cancellation before results and subsequent connection reuse.
README.md	Explains cancellation delays and timeout behavior.
lib/​msnodesqlv8/​request.js	Defers cancellation of queries held before their first result set.

_{Review effort: Balanced}