Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit dc57173

Browse filesBrowse files
Han5991sxa
authored andcommitted
stream: fix Writable.toWeb() hang on synchronous drain
A race condition in the Writable.toWeb() adapter caused the stream to hang if the underlying Node.js Writable emitted a 'drain' event synchronously during a write() call. This often happened when highWaterMark was set to 0. By checking writableNeedDrain immediately after a backpressured write, the adapter now correctly detects if the stream has already drained, resolving the backpressure promise instead of waiting indefinitely for an event that has already occurred. Fixes: #61145 Signed-off-by: sangwook <rewq5991@gmail.com> PR-URL: #61197 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent c20aa4c commit dc57173
Copy full SHA for dc57173

2 files changed

+26Lines changed: 26 additions & 0 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/webstreams/adapters.js‎

Copy file name to clipboardExpand all lines: lib/internal/webstreams/adapters.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@ function newWritableStreamFromStreamWritable(streamWritable, options = kEmptyObj
232232
}
233233
if (streamWritable.writableNeedDrain || !streamWritable.write(chunk)) {
234234
backpressurePromise = PromiseWithResolvers();
235+
if (!streamWritable.writableNeedDrain) {
236+
backpressurePromise.resolve();
237+
}
235238
return SafePromisePrototypeFinally(
236239
backpressurePromise.promise, () => {
237240
backpressurePromise = undefined;
Collapse file

‎test/parallel/test-whatwg-webstreams-adapters-to-writablestream.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-whatwg-webstreams-adapters-to-writablestream.js
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,26 @@ class TestWritable extends Writable {
197197
assert.strictEqual(chunk, arrayBuffer);
198198
}));
199199
}
200+
201+
{
202+
// Test that the stream doesn't hang when the underlying Writable
203+
// emits 'drain' synchronously during write().
204+
// Fixes: https://github.com/nodejs/node/issues/61145
205+
const writable = new Writable({
206+
write(chunk, encoding, callback) {
207+
callback();
208+
},
209+
});
210+
211+
// Force synchronous 'drain' emission during write()
212+
// to simulate a stream that doesn't have Node.js's built-in kSync protection.
213+
writable.write = function(chunk) {
214+
this.emit('drain');
215+
return false;
216+
};
217+
218+
const writableStream = newWritableStreamFromStreamWritable(writable);
219+
const writer = writableStream.getWriter();
220+
writer.write(new Uint8Array([1, 2, 3])).then(common.mustCall());
221+
writer.write(new Uint8Array([4, 5, 6])).then(common.mustCall());
222+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.