Skip to content

fix(stream): gate no-size read() head-chunk behavior on flowing mode#5031

Closed
proggeramlug wants to merge 1 commit into
mainfrom
fix/readable-read-flowing-gate
Closed

fix(stream): gate no-size read() head-chunk behavior on flowing mode#5031
proggeramlug wants to merge 1 commit into
mainfrom
fix/readable-read-flowing-gate

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Summary

Unbreaks cargo-test on main (every open PR is currently red on it).

PR #5017 changed Readable.read() with no size to return only the head chunk of the internal buffer unconditionally. Node's howMuchToRead(NaN) only does that when the stream is flowing:

if (NumberIsNaN(n)) {
  if (state.flowing && state.length) return state.buffer.first().length;
  return state.length; // paused: drain the whole buffer
}

A paused stream drains the entire buffer and returns it as a single value, which is exactly what the two now-failing unit tests assert:

  • node_stream::tests_extra::readable_unshift_prepends_chunk_and_returns_hwm_signal
  • node_stream::tests_extra::readable_unshift_after_eof_before_end_prepends_chunk

(push('world'); unshift('hello '); read()'hello world', verified against real Node.)

Fix

read_stream_available_default keeps the #5017 head-chunk path when readable_is_flowing(stream) and restores the pre-#5017 full-drain concat path (drain_whole_buffer, including the string-encoding branch) when paused. Sized read(n) is untouched.

Validation

  • cargo test --release -p perry-runtime --lib node_stream → 68 passed, 0 failed (both previously-failing tests pass).
  • Full perry-runtime --lib suite: only the two known macOS-local pre-existing failures remain (date full_year, Date.prototype.toJSON); both also fail on unmodified origin/main locally and are green in Linux CI.

Note: the remaining main reds are tracked separately — #5029 (gc_write_barrier_stress) and #5030 (h1_buffer_alias_negative); fixes in flight.

No version bump / changelog per external-PR convention — maintainer folds metadata at merge.

…1545 follow-up)

PR #5017 made read() with no size return only the head chunk of the
internal buffer unconditionally. Node's howMuchToRead(NaN) only does
that when the stream is FLOWING (state.flowing && state.length); a
paused stream drains the entire buffer and returns it as one value.

This broke two cargo-test unit tests on main (unshift prepend tests
expect 'hello world' from a paused read()) and with it every PR's
required cargo-test check.

Fix: keep the head-chunk path for flowing streams, restore the
full-drain concat path (drain_whole_buffer) for paused streams.
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Superseded by #5033 (same commit, stacked with the #5030 fix and the #5029 test gating so all required checks can pass together).

@proggeramlug proggeramlug deleted the fix/readable-read-flowing-gate branch June 12, 2026 06:45
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.

1 participant