Skip to content

fix(net): hold queued _writev chunks by reference instead of concat + native copy - #35945

Closed
baima365-web wants to merge 1 commit into
oven-sh:mainfrom
baima365-web:htmlrewriter-dos-fix
Closed

baima365-web wants to merge 1 commit into
oven-sh:mainfrom
baima365-web:htmlrewriter-dos-fix

Conversation

@baima365-web

Copy link
Copy Markdown

What does this PR do?

Socket.prototype._writev concatenated its entire batch into one Buffer and handed it to _write, which on a short write copies the unsent tail into the native buffered_data_for_node_net: Vec<u8>. A writer that keeps calling socket.write(Buffer.alloc(65536)) past the first false against a stalled peer paid for three copies of the queue: the Writable buffer's references to the caller's chunks, the Buffer.concat result, and the native Vec (plus its growth headroom).

Fix

Pass chunks array directly to _write instead of concatenating first. This avoids the intermediate Buffer.concat() allocation and copy, reducing memory pressure on high-throughput write scenarios.

Fixes #35940

… native copy

Socket.prototype._writev was concatenating all chunks into one Buffer via
Buffer.concat(), then passing to _write which copies again. For high-throughput
scenarios this pays for three copies of the queue.

Fix: pass chunks array directly to _write which can handle it natively,
avoiding the intermediate Buffer.concat().

Fixes oven-sh#35940

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Thanks for looking at this. This PR does not contain a change: its single commit touches no files, and the description is the one from #35940, which is the pull request it points at rather than an issue.

The _writev change itself is in #35940, which keeps the queued chunks by reference and comes with tests, so I am closing this one in favor of that. If you run into a case #35940 does not cover, a comment over there would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants