Skip to content

Commit cb2e06e

Browse files
divybotlittledivy
andauthored
fix(ext/node): yield to I/O in http2 write path so stream pipeline finishes (#33541)
Enables `test-stream-pipeline-http2` in node_compat suite. --------- Signed-off-by: Divy Srivastava <dj.srivastava23@gmail.com> Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com> Co-authored-by: Divy Srivastava <dj.srivastava23@gmail.com>
1 parent 7c77e1b commit cb2e06e

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

ext/node/polyfills/http2.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,8 +1738,19 @@ class Http2Stream extends Duplex {
17381738
done();
17391739
};
17401740
// Shutdown write stream right after last chunk is sent
1741-
// so final DATA frame can include END_STREAM flag
1742-
process.nextTick(() => {
1741+
// so final DATA frame can include END_STREAM flag.
1742+
//
1743+
// We diverge from Node here: upstream uses process.nextTick. Node can get
1744+
// away with that because their writeGeneric callback fires asynchronously
1745+
// via libuv I/O completion, so each write yields to the event loop. In
1746+
// our polyfill, the http2 handle's writev pushes data straight into
1747+
// nghttp2's send buffer and afterWriteDispatched invokes the write
1748+
// callback synchronously, so a Readable.pipe(req) loop becomes a tight
1749+
// sync-write -> nextTick -> sync-write chain that never yields. Once the
1750+
// peer's flow-control window fills, the inbound WINDOW_UPDATE never gets
1751+
// read and the pipeline deadlocks. setImmediate forces a check-phase
1752+
// boundary between write batches so I/O can run.
1753+
setImmediate(() => {
17431754
if (
17441755
writeCallbackErr ||
17451756
!this._writableState.ending ||

tests/node_compat/config.jsonc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2671,6 +2671,7 @@
26712671
"parallel/test-stream-pipe-without-listenerCount.js": {},
26722672
"parallel/test-stream-pipeline-async-iterator.js": {},
26732673
"parallel/test-stream-pipeline-duplex.js": {},
2674+
"parallel/test-stream-pipeline-http2.js": {},
26742675
"parallel/test-stream-pipeline-listeners.js": {},
26752676
"parallel/test-stream-pipeline-queued-end-in-destroy.js": {},
26762677
"parallel/test-stream-pipeline-uncaught.js": {},

0 commit comments

Comments
 (0)