Skip to content

stream: fix drop-newest behavior in share()#64417

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-share-drop-newest
Open

stream: fix drop-newest behavior in share()#64417
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-share-drop-newest

Conversation

@trivikr

@trivikr trivikr commented Jul 11, 2026

Copy link
Copy Markdown
Member

Fixes: #64416

Fixes share() with backpressure: 'drop-newest' so upstream pull
results are discarded when the shared buffer reaches highWaterMark.

Previously, #waitForBufferSpace() returned true for drop-newest,
causing the pulled batch to be appended to the full buffer. The buffer
could therefore grow beyond highWaterMark, and stalled consumers
eventually received every batch.

The async implementation now pulls and discards new results while the
buffer is full. The existing test has been updated to verify that the
buffer remains bounded and dropped batches are not delivered.

Discard upstream pull results when the shared buffer has reached its
highWaterMark instead of allowing the buffer to grow beyond its limit.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
@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 Jul 11, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2026
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.23%. Comparing base (ab41cf0) to head (baae906).

Files with missing lines Patch % Lines
lib/internal/streams/iter/share.js 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #64417   +/-   ##
=======================================
  Coverage   90.23%   90.23%           
=======================================
  Files         741      741           
  Lines      241339   241339           
  Branches    45464    45474   +10     
=======================================
+ Hits       217766   217780   +14     
+ Misses      15132    15122   -10     
+ Partials     8441     8437    -4     
Files with missing lines Coverage Δ
lib/internal/streams/iter/share.js 84.99% <71.42%> (+0.26%) ⬆️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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. request-ci Add this label to start a Jenkins CI on a PR. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream/iter: share() does not discard upstream results with "drop-newest"

2 participants