quic: remove unused fin flag from blob reader wakeup

Project: nodejs/node | Language: JavaScript, C++ | Concepts: refactoring, data-structures, error-handling | Difficulty: mid
Link: https://github.com/nodejs/node/pull/65315

The problem

The QUIC blob reader's wakeup mechanism included a boolean "fin" flag to signal end-of-stream (EOS) to the JavaScript iterator. However, this flag was redundant: the iterator always had to pull again after a wakeup to determine stream state, and the "fin" information was already available from the pull channel itself. The code handling this flag was dead and unnecessarily complicated the wakeup logic.

Why it's non-trivial

A naive fix might just remove the flag in one place, but the flag was threaded through both JavaScript and C++ layers, including the wakeup callback signature and call sites. Additionally, the wakeup logic had a special bypass for EOS that needed careful analysis to ensure its removal wouldn't introduce subtle bugs like missed wakeups or stalled iterators.

The approach

1. Remove the fin state from the JS wakeup callback and the iterator loop

-  let fin = false;
-  reader.setWakeup((setfin) => {
-    fin ||= setfin;
+  reader.setWakeup(() => {
     immediate ??= setImmediate(() => {
       immediate = undefined;
       wakeup.resolve?.();

Annotation:
The JavaScript side no longer expects a boolean argument from the wakeup callback. The local fin variable and logic to accumulate its state are deleted, simplifying the callback to a no-argument function.

2. Remove the dead branch in the iterator that used fin

-        // If the wakeup was triggered by FIN (EndReadable), the DataQueue
-        // is capped. Continue the loop to pull again -- the next pull will
-        // return EOS. Without this, a race between the data notification
-        // and the FIN notification can leave the iterator waiting for a
-        // wakeup that will never come.
-        if (fin) continue;

Annotation:
This block handled the case where a wakeup was triggered by EOS. Since the iterator always needs to pull again after a wakeup, and EOS is now reliably detected by the next pull, this branch is unnecessary and safely removed.

3. Remove the fin parameter from the C++ wakeup notifier

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

Annotation:
The C++ NotifyPull method no longer takes a boolean argument, and the logic for bypassing notification suppression on EOS is removed. All wakeups are now coalesced the same way, reducing complexity and potential for subtle bugs.

4. Update the header and call sites to match the new signature

-    void NotifyPull(bool fin = false);
+    void NotifyPull();

Annotation:
The header file is updated to remove the fin parameter from the method signature.

-  if (reader_) reader_->NotifyPull(true);
+  if (reader_) reader_->NotifyPull();

Annotation:
The call site in the QUIC stream code no longer passes the true argument, reflecting the new method signature.

From the review thread

No notable review discussion on this PR.

Takeaways

• Remove dead or redundant state when a single source of truth is sufficient; avoid duplicating information across channels.
• When simplifying cross-language interfaces, ensure all layers (callers, callbacks, and documentation) are updated consistently to prevent mismatches.
• Coalescing notifications can reduce unnecessary work and complexity, but always analyze edge cases (like end-of-stream) to ensure no required signals are lost.