Skip to content

Commit 2e34a6f

Browse files
committed
http2: settle pending write callbacks on stream destroy
When an HTTP/2 stream is destroyed while a write is still in flight, nghttp2 may have already handed the data off to the socket and never report the write's completion once the stream or session tears down. In that case the callback that resolves the Writable write is never invoked, so the Writable stays stuck and the event loop never drains, causing test-http2-close-while-writing to time out on macOS. In Http2Stream._destroy, settle any write that is still in flight by invoking its callback with the destroy error and resetting writePending. A later native completion callback is then a no-op because writeCb is null and writePending is 0, so the settle is idempotent. Passing the error (rather than null) also drains any buffered writes. Fixes: #58252 Signed-off-by: Matteo Collina <hello@matteocollina.com> Assisted-by: pi-coding-agent
1 parent 6f41e41 commit 2e34a6f

2 files changed

Lines changed: 21 additions & 2 deletions

File tree

‎lib/internal/http2/core.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2721,6 +2721,15 @@ class Http2Stream extends Duplex {
27212721
});
27222722
}
27232723
}
2724+
2725+
// A write in flight may never get its native completion callback once the
2726+
// stream tears down; settle it so the Writable can clean up.
2727+
if (state.writeCb !== null) {
2728+
const writeCb = state.writeCb;
2729+
state.writeCb = null;
2730+
state.writePending = 0;
2731+
writeCb(err);
2732+
}
27242733
callback(err);
27252734
}
27262735
// The Http2Stream can be destroyed if it has closed and if the readable

‎test/parallel/test-http2-close-while-writing.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,21 @@ server.on('session', common.mustCall(function(session) {
2929
stream.on('error', common.mustCall((err) => {
3030
assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_ABORTED');
3131
}));
32-
stream.resume();
32+
33+
// Every write dispatched before close must have its callback invoked.
34+
let writes = 0;
35+
let writeCallbacks = 0;
3336
stream.on('data', function() {
34-
this.write(Buffer.alloc(1));
37+
writes++;
38+
this.write(Buffer.alloc(1), () => {
39+
writeCallbacks++;
40+
});
3541
process.nextTick(() => client_stream.destroy());
3642
});
43+
stream.on('close', common.mustCall(() => {
44+
assert.strictEqual(writeCallbacks, writes);
45+
}));
46+
stream.resume();
3747
}));
3848
}));
3949

0 commit comments

Comments
 (0)