quic: retain readers for readable streams - #66407
GiHoon1123 wants to merge 1 commit into
Conversation
Create readers when readable QuicStream instances are constructed so data received before the first async iterator pull survives native stream cleanup. Add a regression test for a peer-initiated unidirectional stream that sends data and FIN before the reader is pulled. Assisted-by: Codex Signed-off-by: GiHoon1123 <rlaejrqo465@naver.com>
|
Review requested:
|
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
|
The recommendations in: advise you should tackle only one issue at a time and that you should not open any new PRs until your first PR has been approved. |
|
Sorry about that - didn't follow the one-issue-at-a-time guideline. Closing this for now and focusing on #64627 first, will reopen once that's merged. |
|
The problem with multiple PRs for first-time contributors is that every step need to be manually approved by a team member, which creates a high workload and slows down the progression of the PRs. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #66407 +/- ##
=======================================
Coverage 90.36% 90.36%
=======================================
Files 792 792
Lines 275564 275584 +20
Branches 52832 52843 +11
=======================================
+ Hits 249016 249042 +26
+ Misses 16965 16957 -8
- Partials 9583 9585 +2
🚀 New features to boost your workflow:
|
Fixes: #66347
When a peer-initiated unidirectional stream sent data and FIN before the consumer performed
its first async iterator pull, the native stream could be cleaned up before the reader was
created. The buffered data was then no longer available to the JavaScript stream.
Create the reader when a readable QuicStream is constructed so the underlying data queue
remains available until the consumer pulls from the iterator. Local unidirectional streams
remain unreadable and do not allocate a reader.
This means every readable stream allocates a reader whether or not it's ever read, instead
of only on first pull. #66347 asked whether lazy allocation was the right call for this;
eager allocation is the answer this PR takes to that question.
Added a regression test covering data received before the first iterator pull.
Tests: