Skip to content

fix(ext/node): drive TLSWrap cycle on JSStream writes to fix deadlock#33914

Merged
nathanwhit merged 1 commit into
denoland:mainfrom
nathanwhit:jsstream-deadlock
May 7, 2026
Merged

fix(ext/node): drive TLSWrap cycle on JSStream writes to fix deadlock#33914
nathanwhit merged 1 commit into
denoland:mainfrom
nathanwhit:jsstream-deadlock

Conversation

@nathanwhit

Copy link
Copy Markdown
Member

For JS-backed streams (TLSSocket wrapping a non-net.Socket Duplex, the mssql/tedious pattern), enc_write_cb (the libuv write completion path) never fires, so cycle() — the only path that fires InvokeQueued for queued cleartext WriteWraps — was only driven indirectly via peer reads. If the writer awaited each cb before the next chunk and the peer waited for all chunks before responding, the second cb never fired and the connection deadlocked.

Mirror Node`s TLSWrap::EncOut + OnStreamAfterWrite loop: pump encrypted bytes to the underlying Duplex, wait for its _write callback, then drive cycle() (the ClearIn + EncOut analog) to push more cleartext, write more encrypted bytes if produced, and fire InvokeQueued. Gating cycle() on the underlying _write callback also propagates backpressure.

Fixes #33907.

For JS-backed streams (TLSSocket wrapping a non-net.Socket Duplex, the
mssql/tedious pattern), enc_write_cb (the libuv write completion path)
never fires, so cycle() — the only path that fires InvokeQueued for
queued cleartext WriteWraps — was only driven indirectly via peer reads.
If the writer awaited each cb before the next chunk and the peer waited
for all chunks before responding, the second cb never fired and the
connection deadlocked.

Mirror Node`s TLSWrap::EncOut + OnStreamAfterWrite loop: pump encrypted
bytes to the underlying Duplex, wait for its _write callback, then drive
cycle() (the ClearIn + EncOut analog) to push more cleartext, write more
encrypted bytes if produced, and fire InvokeQueued. Gating cycle() on
the underlying _write callback also propagates backpressure.

Fixes denoland#33907.
@nathanwhit
nathanwhit enabled auto-merge (squash) May 7, 2026 20:26
@nathanwhit
nathanwhit merged commit 06c1659 into denoland:main May 7, 2026
136 checks passed
littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 2026
…denoland#33914)

For JS-backed streams (TLSSocket wrapping a non-net.Socket Duplex, the
mssql/tedious pattern), enc_write_cb (the libuv write completion path)
never fires, so cycle() — the only path that fires InvokeQueued for
queued cleartext WriteWraps — was only driven indirectly via peer reads.
If the writer awaited each cb before the next chunk and the peer waited
for all chunks before responding, the second cb never fired and the
connection deadlocked.

Mirror Node`s TLSWrap::EncOut + OnStreamAfterWrite loop: pump encrypted
bytes to the underlying Duplex, wait for its _write callback, then drive
cycle() (the ClearIn + EncOut analog) to push more cleartext, write more
encrypted bytes if produced, and fire InvokeQueued. Gating cycle() on
the underlying _write callback also propagates backpressure.

Fixes denoland#33907.

<!--
IMPORTANT: If you used AI tools (e.g. Copilot, ChatGPT, Claude, Cursor,
etc.)
to help write this PR, you MUST disclose it in the PR description. PRs
will be
rejected if there is suspicion of undisclosed AI usage.

Before submitting a PR, please read
https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(ext/net): fix race condition in TCP listener
    - docs(runtime): update permissions API docstrings
    - feat(cli): add --env-file flag to `deno run`
    - refactor(ext/node): simplify Buffer.from() implementation
    - test(ext/fs): add spec tests for symlink resolution

  Examples of bad title:
    - fix denoland#7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `./x fmt` passes without changing files.
5. Ensure `./x lint` passes.
6. If you used AI tools to help write this PR, you MUST disclose it
below.
   PRs will be rejected if there is suspicion of undisclosed AI usage.
7. Open as a draft PR if your work is still in progress. The CI won't
run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench'
label.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:tls JSStreamSocket deadlocks on multi-packet writes (affects npm:mssql/tedious)

1 participant