Skip to content

Don't set DATA send flag for zero-length send requests - #6248

Open
Tanishq Sharma (Tani-shar) wants to merge 3 commits into
microsoft:mainfrom
Tani-shar:fix-zero-length-send-wedge
Open

Don't set DATA send flag for zero-length send requests#6248
Tanishq Sharma (Tani-shar) wants to merge 3 commits into
microsoft:mainfrom
Tani-shar:fix-zero-length-send-wedge

Conversation

@Tani-shar

@Tani-shar Tanishq Sharma (Tani-shar) commented Aug 21, 2026

Copy link
Copy Markdown

Fixes #6243.

  1. The change: skip QuicSendSetStreamSendFlag(..., QUIC_STREAM_SEND_FLAG_DATA, ...) in QuicStreamSendFlush when SendRequest->TotalLength == 0.

  2. Why this option over the other two in the issue: clearing DATA when a flush finds nothing sendable would also fire when the peer hasn't allowed the stream or when flow control blocks it, QuicStreamSendCanWriteDataFrames treats those as separate branches from "no unsent data" , so that fix would wedge streams a different way. Rejecting zero-length sends outright changes public API behavior.

  3. FIN is unaffected: QuicStreamSendShutdown sets QUIC_STREAM_SEND_FLAG_FIN directly on the graceful path, so a zero-length send with FIN still works.

  4. Verified: repro from the issue reproduces on 4984c21 (2.7.0), PASS after the fix, SKIP_EMPTY_SEND=1 still PASS, and SEND_COMPLETE still fires for the zero-length request.

Update: added a regression test and a second guard.

  1. Guard 1 (above) skips setting DATA for zero-length requests in
    QuicStreamSendFlush.

  2. Guard 2 clears a stale DATA flag in the stream write path when the flag is
    set but QuicStreamHasPendingStreamData returns false. This is the third
    option from the issue, but keyed on QuicStreamHasPendingStreamData rather
    than QuicStreamSendCanWriteDataFrames — the former compares send offsets
    only, so it doesn't fire when data is pending but blocked by flow control or
    by the peer not yet allowing the stream. That's the concern raised in point 2
    above.

  3. Regression test Misc.StreamZeroLengthSend fails on main at 4984c21 and
    passes with the fix. Each guard breaks the wedge independently, verified by
    disabling one at a time.

@Tani-shar
Tanishq Sharma (Tani-shar) requested a review from a team as a code owner August 21, 2026 17:04
@Tani-shar

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.94%. Comparing base (4984c21) to head (e769795).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/core/stream_send.c 75.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (75.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6248      +/-   ##
==========================================
- Coverage   85.28%   84.94%   -0.34%     
==========================================
  Files          60       60              
  Lines       18973    18977       +4     
==========================================
- Hits        16181    16120      -61     
- Misses       2792     2857      +65     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Zero-length StreamSend leaves QUIC_STREAM_SEND_FLAG_DATA set without a flush, permanently wedging the stream

1 participant