Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 2e86855

Browse files
trivikraduh95
authored andcommitted
stream: avoid duplicate writes in toWritable
PushWriter can return false after accepting a chunk when block backpressure is active. Teach the classic Writable adapter to treat that case as accepted backpressure instead of retrying through the async write path. Fixes: #63359 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63360 Backport-PR-URL: #64675Fixes: #63359 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 88a3392 commit 2e86855

4 files changed

Lines changed: 119 additions & 6 deletions

File tree

β€Žlib/internal/streams/iter/classic.jsβ€Ž

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
const{
5858
toAsyncStreamable: kToAsyncStreamable,
5959
kValidatedSource,
60+
kSyncWriteAccepted,
6061
drainableProtocol,
6162
}=require('internal/streams/iter/types');
6263

@@ -764,13 +765,41 @@ function toWritable(writer) {
764765
consthasEndSync=hasEnd&&
765766
typeofwriter.endSync==='function';
766767
consthasFail=typeofwriter.fail==='function';
768+
consthasSyncWriteAccepted=
769+
typeofwriter[kSyncWriteAccepted]==='function';
767770

768-
// Try-sync-first pattern: attempt the synchronous method and
769-
// fall back to the async method if it returns false (indicating
770-
// the sync path was not accepted) or throws. When the sync path
771-
// succeeds, the callback is deferred via queueMicrotask to
772-
// preserve the async resolution contract that Writable internals
773-
// expect from _write/_writev/_final callbacks.
771+
functionsyncWriteAccepted(){
772+
returnhasSyncWriteAccepted&&writer[kSyncWriteAccepted]();
773+
}
774+
775+
functionfinishAfterSyncBackpressure(cb){
776+
letondrain;
777+
try{
778+
if(typeofwriter[drainableProtocol]==='function'){
779+
ondrain=writer[drainableProtocol]();
780+
}
781+
}catch(err){
782+
cb(err);
783+
return;
784+
}
785+
if(ondrain!==null&&ondrain!==undefined){
786+
PromisePrototypeThen(ondrain,(drained)=>{
787+
if(drained===false){
788+
cb(newERR_INVALID_STATE.TypeError('Stream closed by consumer'));
789+
return;
790+
}
791+
cb();
792+
},cb);
793+
return;
794+
}
795+
queueMicrotask(cb);
796+
}
797+
798+
// Try-sync-first pattern: attempt the synchronous method and fall back to the
799+
// async method if it returns false without accepting the data, or if it
800+
// throws. When the sync path succeeds, the callback is deferred via
801+
// queueMicrotask to preserve the async resolution contract that Writable
802+
// internals expect from _write/_writev/_final callbacks.
774803

775804
function_write(chunk,encoding,cb){
776805
constbytes=typeofchunk==='string' ?
@@ -781,6 +810,11 @@ function toWritable(writer) {
781810
queueMicrotask(cb);
782811
return;
783812
}
813+
if(syncWriteAccepted()){
814+
// The chunk was accepted; false only signaled backpressure.
815+
finishAfterSyncBackpressure(cb);
816+
return;
817+
}
784818
}catch{
785819
// Sync path threw -- fall through to async.
786820
}
@@ -805,6 +839,11 @@ function toWritable(writer) {
805839
queueMicrotask(cb);
806840
return;
807841
}
842+
if(syncWriteAccepted()){
843+
// The chunks were accepted; false only signaled backpressure.
844+
finishAfterSyncBackpressure(cb);
845+
return;
846+
}
808847
}catch{
809848
// Sync path threw -- fall through to async.
810849
}

β€Žlib/internal/streams/iter/push.jsβ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const {
3232

3333
const{
3434
drainableProtocol,
35+
kSyncWriteAccepted,
3536
kSyncWriteAcceptedOnFalse,
3637
}=require('internal/streams/iter/types');
3738

@@ -545,11 +546,16 @@ class PushQueue {
545546

546547
classPushWriter{
547548
#queue;
549+
#syncWriteAccepted =false;
548550

549551
constructor(queue){
550552
this.#queue =queue;
551553
}
552554

555+
[kSyncWriteAccepted](){
556+
returnthis.#syncWriteAccepted;
557+
}
558+
553559
[drainableProtocol](){
554560
constdesired=this.desiredSize;
555561
if(desired===null)returnnull;
@@ -589,19 +595,23 @@ class PushWriter {
589595
}
590596

591597
writeSync(chunk){
598+
this.#syncWriteAccepted =false;
592599
constbytes=toUint8Array(chunk);
593600
constresult=this.#queue.writeSync([bytes]);
594601
if(!result&&this.#queue.backpressurePolicy==='block'&&
595602
this.#queue.desiredSize===0){
596603
// Block policy: force-enqueue and return false as backpressure signal.
597604
// Data IS accepted; false tells caller to slow down.
598605
this.#queue.forceEnqueue([bytes]);
606+
this.#syncWriteAccepted =true;
599607
returnfalse;
600608
}
609+
this.#syncWriteAccepted =result;
601610
returnresult;
602611
}
603612

604613
writevSync(chunks){
614+
this.#syncWriteAccepted =false;
605615
if(!ArrayIsArray(chunks)){
606616
thrownewERR_INVALID_ARG_TYPE('chunks','Array',chunks);
607617
}
@@ -610,8 +620,10 @@ class PushWriter {
610620
if(!result&&this.#queue.backpressurePolicy==='block'&&
611621
this.#queue.desiredSize===0){
612622
this.#queue.forceEnqueue(bytes);
623+
this.#syncWriteAccepted =true;
613624
returnfalse;
614625
}
626+
this.#syncWriteAccepted =result;
615627
returnresult;
616628
}
617629

β€Žlib/internal/streams/iter/types.jsβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ const kValidatedTransform = Symbol('kValidatedTransform');
6464
*/
6565
constkValidatedSource=Symbol('kValidatedSource');
6666

67+
/**
68+
* Internal sentinel for writers whose sync write methods can return false
69+
* after accepting data as a backpressure signal.
70+
*/
71+
constkSyncWriteAccepted=Symbol('kSyncWriteAccepted');
72+
73+
/**
74+
* Internal sentinel for writers whose sync write methods may return false
75+
* after accepting data when backpressure is applied. Such writers must expose
76+
* desiredSize so callers can distinguish accepted backpressure from a sync
77+
* write that was not performed.
78+
*/
6779
constkSyncWriteAcceptedOnFalse=Symbol('kSyncWriteAcceptedOnFalse');
6880

6981
module.exports={
7082
broadcastProtocol,
7183
drainableProtocol,
84+
kSyncWriteAccepted,
7285
kSyncWriteAcceptedOnFalse,
7386
kValidatedSource,
7487
kValidatedTransform,

β€Žtest/parallel/test-stream-iter-writable-from.jsβ€Ž

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,53 @@ async function testRoundTrip() {
335335
assert.strictEqual(result,data);
336336
}
337337

338+
// =============================================================================
339+
// PushWriter writeSync false accepted as backpressure is not retried
340+
// =============================================================================
341+
342+
asyncfunctiontestPushWriterBlockBackpressureNoDuplicate(){
343+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
344+
constwritable=toWritable(writer);
345+
346+
awaitnewPromise((resolve,reject)=>{
347+
writable.write('a',(err)=>{
348+
if(err)reject(err);
349+
elseresolve();
350+
});
351+
});
352+
353+
writable.write('b');
354+
writable.end();
355+
356+
constresult=awaittext(readable);
357+
assert.strictEqual(result,'ab');
358+
}
359+
360+
// =============================================================================
361+
// PushWriter writevSync false accepted as backpressure is not retried
362+
// =============================================================================
363+
364+
asyncfunctiontestPushWriterBlockBackpressureWritevNoDuplicate(){
365+
const{ writer, readable }=push({highWaterMark: 1,backpressure: 'block'});
366+
constwritable=toWritable(writer);
367+
368+
awaitnewPromise((resolve,reject)=>{
369+
writable.write('a',(err)=>{
370+
if(err)reject(err);
371+
elseresolve();
372+
});
373+
});
374+
375+
writable.cork();
376+
writable.write('b');
377+
writable.write('c');
378+
writable.uncork();
379+
writable.end();
380+
381+
constresult=awaittext(readable);
382+
assert.strictEqual(result,'abc');
383+
}
384+
338385
// =============================================================================
339386
// Multiple sequential writes
340387
// =============================================================================
@@ -590,6 +637,8 @@ Promise.all([
590637
testWriteThrowsSyncPropagation(),
591638
testEndThrowsSyncPropagation(),
592639
testRoundTrip(),
640+
testPushWriterBlockBackpressureNoDuplicate(),
641+
testPushWriterBlockBackpressureWritevNoDuplicate(),
593642
testSequentialWrites(),
594643
testSyncCallbackDeferred(),
595644
testMinimalWriter(),

0 commit comments

Comments
Β (0)