Skip to content

Commit dc57173

Browse 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

2 files changed

Lines changed: 26 additions & 0 deletions

File tree

‎lib/internal/webstreams/adapters.js‎

Lines 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
returnSafePromisePrototypeFinally(
236239
backpressurePromise.promise,()=>{
237240
backpressurePromise=undefined;

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

Lines 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+
constwritable=newWritable({
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+
returnfalse;
216+
};
217+
218+
constwritableStream=newWritableStreamFromStreamWritable(writable);
219+
constwriter=writableStream.getWriter();
220+
writer.write(newUint8Array([1,2,3])).then(common.mustCall());
221+
writer.write(newUint8Array([4,5,6])).then(common.mustCall());
222+
}

0 commit comments

Comments
 (0)