Repository navigation
fix(tls): start reads on adopted paused duplex transports - #38
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Paused transports handed to TLS now complete their handshake. Previously,
tls.connect({ socket: duplex })and a paused HTTP CONNECT socket injected intotls.Servercould wait indefinitely even though the encrypted bytes were buffered or available.The stream upgrade attached TLS listeners but left the underlying Duplex paused. Its native
resume_streamhook is currently a no-op, so reading the outer TLSSocket does not start that transport. Resume the transport after assigning the TLS handle and installing all data/end/drain/close/error listeners, in all four stream-upgrade paths. Native fd adoption stays unchanged. This matches Node 24: TLS takes ownership and starts reading, while the previous owner can keep bytes paused until adoption.Validation:
e8105cb349at the explicit handshake deadline; all pass on Node 24.21.0 and the patched release build (268 ms combined).--expose-internalsfor surrounding suites: TLS: 461 passed, 32 skipped, 6 TODO; net: 266 passed, 37 skipped. HTTP: 1,662 passed, 29 skipped, 3 TODO, with one existing Node-reference test failure innode-http-req-complete.test.ts; the same test fails on the baseline and with Node 24. All Bun assertions in that file pass.OpenClaw consumer proof uses detached main
95ed4ee478cd8, the patched binary first on PATH,OPENCLAW_VITEST_RUNTIME=bun, one worker, private 0700 TMPDIR, disabled disposable compile caching, 15-second test/hook deadlines, and a 300-second outer file guard. The three full proxy files pass:proxy-server.test.ts38/38 in 38.76 s;proxy-server.websocket.test.ts18/18 in 29.48 s;proxy-server.lifecycle.test.ts38/38 in 38.20 s. Unpatched: main proxy file hit the 300-second outer guard (306.50 s including shutdown); websocket finished with 5 passed / 13 failed in 250.14 s; lifecycle hit the outer guard (310.03 s including shutdown). Patched: all 94 tests pass. These are failure-removal checks on a shared Darwin host, not isolated throughput benchmarks.W15 attributed roughly 900–937 seconds of aggregate invocation-wall opportunity to this defect in the old Linux baseline (secrets config: Node 92.242 s, Bun 1029.090 s). That estimate is not claimed as measured patched savings.
Upstream search: no exact initial-read fix found among oven-sh/bun issue/PR searches for TLS, paused Duplex and CONNECT. Open oven-sh#42332 concerns ongoing Duplex backpressure; open oven-sh#38076 concerns server native-adoption fallbacks; merged oven-sh#39830 concerns net pause/read semantics. None supplies this missing initial stream resume.