Skip to content

fix(utils): use unshift in byteStream unwrap#3507

Merged
tabcat merged 9 commits into
libp2p:mainfrom
tabcat:fix/byte-stream-unshift
May 15, 2026
Merged

fix(utils): use unshift in byteStream unwrap#3507
tabcat merged 9 commits into
libp2p:mainfrom
tabcat:fix/byte-stream-unshift

Conversation

@tabcat
Copy link
Copy Markdown
Member

@tabcat tabcat commented May 15, 2026

Description

byteStream unwrap calls stream.push(readBuffer) which appends. If the
underlying stream's readBuffer is non-empty at unwrap time, the
byteStream's older leftover bytes end up after the newer ones, corrupting
the byte order seen by the next consumer.

This happens when:

  • the stream is paused, so dispatchReadBuffer() short-circuits and
    incoming bytes accumulate in stream.readBuffer instead of reaching
    the byteStream listener;
  • external code calls stream.push(...) and then unwrap() on the same
    tick, since push defers dispatch via setTimeout.

Switching to stream.unshift(readBuffer) prepends, preserving FIFO order.

Notes & open questions

The added test reproduces the paused-stream trigger deterministically.
The synchronous-push trigger reduces to the same root cause and is
covered by the same fix.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

lodekeeper and others added 9 commits March 21, 2026 17:06
Add a try-catch around sendData() in processSendQueue() to catch
StreamStateError when the underlying transport closes between a drain
event and the send attempt. This prevents the error from propagating
as an uncaught exception.

Also add a writeStatus guard in continueSendingOnDrain to skip
processing when the stream has fully closed.

Fixes libp2p#3415
Demonstrates that byteStream unwrap appends its buffered bytes after
data already queued on the underlying stream, corrupting byte order.
# Conflicts:
#	packages/utils/src/abstract-message-stream.ts
#	packages/utils/test/stream-utils-test.spec.ts
push appends, so any bytes already buffered on the underlying stream
end up ahead of the byteStream's older leftover bytes. unshift prepends
and keeps the FIFO order.
@tabcat tabcat changed the title fix(utils): use unshift in byteStream unwrap to preserve byte order fix(utils): use unshift in byteStream unwrap May 15, 2026
@tabcat tabcat marked this pull request as ready for review May 15, 2026 12:29
@tabcat tabcat requested a review from a team as a code owner May 15, 2026 12:29
@tabcat tabcat merged commit 9eb27be into libp2p:main May 15, 2026
49 of 50 checks passed
@tabcat tabcat mentioned this pull request May 15, 2026
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.

3 participants