Skip to content

fix: connection.Read copies directly from buffer nodes#413

Open
xiaost wants to merge 1 commit intomainfrom
fix/conn-read
Open

fix: connection.Read copies directly from buffer nodes#413
xiaost wants to merge 1 commit intomainfrom
fix/conn-read

Conversation

@xiaost
Copy link
Contributor

@xiaost xiaost commented Mar 2, 2026

The old implementation called Next(n) then copy(p, src). For multi-node reads, Next allocates an intermediate buffer and copies data into it, then copy duplicates it again into p — a double copy plus an unnecessary allocation.

Additionally, Next+Release in Read could invalidate zero-copy slices that the caller still holds from prior Reader.Next calls, since Release frees all consumed nodes from head to read indiscriminately.

Replace Next+copy+Release with readTo, which:

  • Copies directly from underlying LinkBuffer nodes into p (single copy)
  • Releases fully-consumed nodes inline only when safe (head == node), meaning no outstanding zero-copy references exist
  • Skips inline release when head != read, leaving nodes for the caller's eventual Release call

Also fix ioReadWriter to embed io.Reader/io.Writer interfaces and add BUG/deprecation comments for ioReader.Read.

@xiaost xiaost requested review from a team as code owners March 2, 2026 09:38
The old implementation called Next(n) then copy(p, src). For multi-node
reads, Next allocates an intermediate buffer and copies data into it,
then copy duplicates it again into p — a double copy plus an unnecessary
allocation.

Additionally, Next+Release in Read could invalidate zero-copy slices
that the caller still holds from prior Reader.Next calls, since Release
frees all consumed nodes from head to read indiscriminately.

Replace Next+copy+Release with readTo, which:
- Copies directly from underlying LinkBuffer nodes into p (single copy)
- Releases fully-consumed nodes inline only when safe (head == node),
  meaning no outstanding zero-copy references exist
- Skips inline release when head != read, leaving nodes for the
  caller's eventual Release call

Also fix ioReadWriter to embed io.Reader/io.Writer interfaces and add
BUG/deprecation comments for ioReader.Read.
}
old := b.read
b.read = b.read.next
if b.head == old {
Copy link
Contributor

Choose a reason for hiding this comment

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

假设 head == read,然后通过 Next 单节点读了 read 的一部分。之后 Read 走到这里,好像会把 read 给 release 掉,导致 Next 读的那部分内存回收了。

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants