Skip to content

stream: avoid duplicated endReadableNT scheduling - #65310

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:stream-dedupe-endreadable-tick
Open

stream: avoid duplicated endReadableNT scheduling#65310
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:stream-dedupe-endreadable-tick

Conversation

@mcollina

Copy link
Copy Markdown
Member

Calling read() on an ended stream multiple times before the microtask queue drains scheduled one endReadableNT tick per call, since the only guard (endEmitted) is set inside the tick itself. A hello-world HTTP server was scheduling it four times per request while dumping the unread request body.

Add a kEndScheduled flag, armed on scheduling and cleared when the tick runs. It must be cleared unconditionally: undestroy() resets endEmitted through the state descriptors but cannot reach this flag, and a stale value would block 'end' after a net.Socket reconnect.

Benchmarks vs main: streams/finished.js readable +10.45% (99.9% confidence), no significant regressions across the streams suite; ~+1-3% on HTTP server benchmarks (10 → 7 nextTicks per request).

Calling read() on an ended stream multiple times before the microtask
queue drains scheduled one endReadableNT tick per call, as the only
guard was endEmitted, which is set inside the tick itself. A hello-world
HTTP server was scheduling it four times per request while dumping the
unread request body.

Introduce a kEndScheduled flag armed when the tick is scheduled and
cleared when it runs. Clearing it unconditionally matters for reused
sockets: undestroy() resets endEmitted through the state descriptors
but cannot reach this flag, and a stale value would block the 'end'
event after a net.Socket reconnect.
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants