quic: remove unused fin flag from blob reader wakeup · nodejs/node@024bef2 · GitHub
Skip to content

Commit 024bef2

Browse files
trivenayaduh95
authored andcommitted
quic: remove unused fin flag from blob reader wakeup
The fin argument threaded from Blob::Reader::NotifyPull to the JS blob reader iterator was dead: the only consumer, `if (fin) continue;`, was the last statement in the loop and behaved identically to falling through. End-of-stream is always discovered by the subsequent pull returning EOS, never via the wakeup label. Remove the flag from the JS iterator, NotifyPull's signature/argv, and the EndReadable call site. The `!fin` coalescing bypass collapses safely because a parked reader always has pull_pending_ == false, so the first wakeup after parking always fires. Refs: #64767 Signed-off-by: Naman Trivedi <trivenay@amazon.com> PR-URL: #65315 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 191ebfb commit 024bef2

4 files changed

Lines changed: 11 additions & 24 deletions

File tree

‎lib/internal/blob.js‎

Lines changed: 1 addition & 9 deletions

‎src/node_blob.cc‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -423,20 +423,16 @@ void Blob::Reader::SetWakeup(const FunctionCallbackInfo<Value>& args) {
423423
reader->wakeup_.Reset(args.GetIsolate(), args[0].As<Function>());
424424
}
425425

426-
void Blob::Reader::NotifyPull(bool fin) {
426+
void Blob::Reader::NotifyPull() {
427427
if (wakeup_.IsEmpty() || !env()->can_call_into_js()) return;
428-
// FIN notifications always fire — they must not be suppressed by
429-
// pull_pending_ because there will be no further notifications to
430-
// wake the iterator. Regular data notifications respect pull_pending_
431-
// to coalesce multiple deliveries within a single packet.
432-
if (!fin && pull_pending_) return;
428+
// Coalesce notifications: if a wakeup is already pending and the reader
429+
// has not yet pulled, skip re-notifying to avoid redundant wakeups
430+
// within a single packet.
431+
if (pull_pending_) return;
433432
pull_pending_ = true;
434433
HandleScope handle_scope(env()->isolate());
435434
Local<Function> fn = wakeup_.Get(env()->isolate());
436-
// Pass fin as the first argument so the JS iterator knows EOS is
437-
// imminent and should pull again without waiting for another wakeup.
438-
Local<Value> argv[] = {v8::Boolean::New(env()->isolate(), fin)};
439-
MakeCallback(fn, 1, argv);
435+
MakeCallback(fn, 0, nullptr);
440436
}
441437

442438
BaseObjectPtr<BaseObject> Blob::BlobTransferData::Deserialize(

‎src/node_blob.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class Blob : public BaseObject {
8282
BaseObjectPtr<Blob> blob);
8383
static void Pull(const v8::FunctionCallbackInfo<v8::Value>& args);
8484
static void SetWakeup(const v8::FunctionCallbackInfo<v8::Value>& args);
85-
void NotifyPull(bool fin = false);
85+
void NotifyPull();
8686

8787
explicit Reader(Environment* env,
8888
v8::Local<v8::Object> obj,

‎src/quic/streams.cc‎

Lines changed: 3 additions & 4 deletions

0 commit comments

Comments
 (0)