[akasakariko]

Summary

A FIN can remain unsent when its DATA is acknowledged in another packet or when retransmitted DATA ends before an acknowledged tail

• track the stream's final size lifecycle in SendBuf as a single FinState
• requeue a lost FIN even when its DATA was acknowledged separately
• keep the stream flushable when only the pending FIN remains, while avoiding resending a FIN that is already in flight
• extend existing tests with the standalone regressions from stream: pending FIN can be stranded after DATA acknowledgement and retransmission #2800 and retain coverage of repeated FIN writes, acknowledged FIN loss and pending FIN cleanup on reset

Testing

Local checks use a generated Cargo.lock and RUSTFLAGS="-D warnings"

• cargo +nightly fmt -- --check
• cargo clippy --locked --features=async,ffi,qlog,rpk,congestion_window_unchecked_available --workspace --all-targets -- -D warnings
• cargo test --locked --all-targets --features=async,ffi,qlog,rpk,congestion_window_unchecked_available --workspace
• cargo test --locked --doc --features=async,ffi,qlog,rpk,congestion_window_unchecked_available --workspace

Fixes #2800

[akasakariko]

@lixing-lx Thanks for offering to help validate! The fix is up in this PR — would you mind running it against your TrustTunnel test setup to confirm the stranded FIN case is resolved?

[lixing-lx]

Thanks @akasakariko. I validated b040feea6e436f0987d44b25274ebcea5869f57c; it resolves both original stranded-FIN reproducers in my setup.

Deterministic comparison (same dependency lock, BoringSSL 5.2.0, Rust 1.99.0): I appended the two original Issue #2800 reproducer functions unchanged in behavior to separate base/head checkouts. On base 4d1a5341a9c71a3c79be1b2abb9611c389f349a7, all four CUBIC/BBRv2 cases fail; on this PR, all four pass. The complete quiche --lib suite on the PR is 1,224 passed / 0 failed / 0 ignored, including those four cases and the PR's additional pending/in-flight/reset regressions. Nightly-2026-09-15 fmt and strict quiche all-target clippy with ffi/qlog and BoringSSL rpk enabled also pass.

TrustTunnel downstream validation: I compiled both the client and the server against this exact PR's quiche production sources, checking the changed production files against the commit. Client/server Cargo target directories are separate. The server uses the source from TrustTunnel's dependency-upgrade PR #155, aefb830b19158d1095024551112408f287b6b1d5, so it accepts quiche 0.30.0 / BoringSSL 5.2.0. It has no earlier local FIN-scheduling patches or other local server transport/control/stream repairs. This is an explicitly labelled experimental peer, not the v1.1.0 official release binary.

H2/H3 smoke tests pass, covering TCP/UDP, authentication and certificate rejection, half-close and concurrent byte-checked transfers. The sustained H3 run uses client BBRv2 with a 32-packet initial window and the server's unchanged default congestion settings, in a Linux arm64 Docker namespace with network=none, no host ports, read-only root and dropped capabilities:

Phase	Duration	Completed 64 KiB transfers with EOF
Stable	1,200.711 s	4,365
Latency	1,201.166 s	3,328
3% QUIC loss + latency/bandwidth limits	1,202.854 s	1,124

Total: 8,817 content-and-EOF-verified transfers / 577,830,912 bytes, no missing EOF or transfer timeout. The same run passes 20 session recreations, cancellation of an old read during a blackhole, bounded blackhole dial failure and subsequent recovery. Kernel UDP receive-buffer/send-buffer error counters remain zero; the acceptance validator checks all phase durations/counts and recreation cases, not just the test harness exit code.

The latency/loss relay uses approximately 80 ms ± 10 ms delay per direction and a 10 Mbps cap per direction. The fault conditions, source fingerprints, binary SHA256 values and per-run logs are retained locally. This confirms the standalone scheduling failures and the controlled TrustTunnel path on this head; it is not a claim of public-network or production-server acceptance.

[akasakariko]

Thanks a lot @lixing-lx for the thorough validation, especially the downstream TrustTunnel run under loss and latency. Really appreciate it

Maintainers, this is ready for review whenever you have a chance. Happy to address any feedback

[akasakariko]

Updated in 38880fe78df4a2b4926dd9e7b529570e2cb03cf3

• Merged current master at 2f21a5ba and resolved the test conflict without dropping either side
• Adapted FIN retransmission to PeerStopSendingState and extended the existing notification lifecycle test to ensure neither pending nor delivered STOP_SENDING notifications allow a FIN to be requeued
• Kept all four existing FIN regression functions unchanged
• Confirmed the two original stranded-FIN cases still fail on current master under both CUBIC and BBRv2 and pass on the updated branch

Local validation on macOS arm64

Check	Result
Quiche default all-target tests	1,241 passed
Quiche FFI/qlog/RPK all-target tests and docs with boring 5.3.0	1,249 tests and 45 doctests passed
Workspace all-target tests with CI features	1,553 passed
Workspace doctests with CI features	66 passed, 1 existing ignored example
Strict Clippy for the workspace and all quiche targets	Passed
Rust 1.88 with boring 4.19.0, FFI/qlog/RPK	All-target check passed, 1,249 tests and 45 doctests passed
cargo +nightly-2026-09-15 fmt -- --check and diff checks	Passed

GitHub now reports the PR as mergeable without conflicts
The new Stable, Nightly and Semgrep workflows require maintainer approval and have not executed, so the results above are local validation only

[Luoxin]

Test data for this PR, from a hysteria2 client built on quiche (CUBIC/BBRv2).

Scenario: bulk upload of 128 MiB over a single HTTP/3-style stream (hysteria2 TCP proxy stream) to a sing-box server, with 20 ms added delay and 5% random packet loss on the server's inbound UDP path (netem). The application writes all the data, then the FIN afterwards. When earlier lost data is retransmitted before the pending FIN has been sent, the FIN is dropped from the send queue and never goes out, so the peer never sees end-of-stream and the upload sits until the 150 s timeout.

Numbers (150 s timeout counts as a hang):

• before the fix: 5 hangs in 46 runs
• with a local fix: 0 hangs in 60 runs
• with the fix from stream: keep a pending FIN schedulable after data is acknowledged #2804 ported onto quiche 0.30.0 (plus the prerequisite af8788e, "track FIN acknowledgements separately", because 0.30.0 completes the send side as soon as data is acked): 0 hangs in 30 runs, median 400 Mbit/s (min 261, max 437)

Our regression tests in the fork (fin_after_all_data_sent_survives_retransmission, lost_fin_only_frame_survives_retransmission, each for cubic and bbr2_gcongestion) pass unchanged on top of #2804. We have adopted #2804 in our fork (lyxamour/quiche, branch hotarubi) in place of our own fix, since it also covers a lost PTO probe and the no-STREAM-after-RESET_STREAM case. Hoping it gets merged upstream.

[akasakariko]

Friendly ping. I have updated the branch onto the latest master, and it merges cleanly. Since the last update, a downstream user has also posted independent test results on this PR and adopted this fix in their fork. Happy to adjust anything if reviewers have feedback. Thank you.

[ghedo]

Thanks a lot for the fix, and sorry it took a while to review this.

Tracking the pending FIN in SendBuf looks like the right approach. The extra state is needed to send the FIN when retransmitted data ends before an acknowledged tail, without resending a FIN that is already in flight. I also confirmed that the two regression tests fail on master and pass with this change.

Most of the requested changes are about keeping the PR minimal.

• The reset checks in retransmit_fin(), stream_lost_fin_is_not_retransmitted_after_reset and the related unit test assertions became redundant with the latest merge from master, which already drops lost STREAM frames on reset streams. For the same reason, please drop the "no STREAM frame after RESET_STREAM" item from the description and commit message.
• The other new tests can mostly be folded into existing ones, as described inline.

I also left a couple of smaller suggestions inline.

[akasakariko]

All review points are addressed in c84374e

• fin_pending and fin_acked are a single FinState enum with Pending, Sent and Acked, only meaningful once fin_off is set
• retransmit_fin() only checks the FIN state, as the loss path already skips reset streams and reset() moves the final size to RESET_STREAM
• is_fin_pending() is pub(crate)
• the new SendBuf tests are folded into send_buf_empty_fin_next and retransmit_returns_zero_when_already_acked with is_flushable() assertions
• send_buf_fin_not_pending_after_shutdown and stream_lost_fin_is_not_retransmitted_after_reset are removed, and the retransmit_fin() assertions in peer_stop_sending_notification_lifecycle are gone
• stream_empty_fin_is_retransmitted gained a fin_with_data case, replacing stream_fin_is_retransmitted_after_data_acked_separately
• stream_fin_is_sent_after_retransmission_before_acked_tail gained a fin_with_tail case, replacing both stream_fin_is_retransmitted_after_data_acked_separately and stream_inflight_fin_is_not_resent_after_data_retransmission
• the description and commit message no longer mention the RESET_STREAM case

The branch is rebased onto current master and is a single commit

Local validation on macOS arm64 with boring 5.4.0

Check	Result
cargo +nightly fmt -- --check	Passed
Workspace all-target Clippy with -D warnings	Passed
Workspace all-target tests with CI features	1,303 passed
Workspace doctests	66 passed, 1 existing ignored example

Both folded regressions still fail on master and pass with this change

[ghedo]

Thanks for the quick update. This looks much better. The FinState enum maps exactly onto the previous booleans, and I checked that the folded regression tests still fail on master and that the fin_with_tail case catches the simpler fix from #2800.

When I checked the folded tests with a few mutations, I found that the tests no longer check that a lost FIN stays acked, that writing the FIN again doesn't schedule another one, or that a reset clears the pending FIN. Sorry, my earlier comments weren't clear that some steps of the removed unit tests had no equivalent in the existing ones. The inline suggestions restore that coverage with a few lines in the existing tests.

I also think the reserve_for_write() change can be dropped, as described inline.

[akasakariko]

Updated in 4393dcf

All four follow-up comments are addressed and replied to inline, and the branch remains a single commit

Local validation on macOS arm64

Check	Result
Workspace all-targets tests with CI features	1,619 passed with both boring 5.4.0 and boring 4.19.0
Workspace doctests	66 passed and 1 existing ignored example with both dependency versions
Workspace all-targets Clippy with -D warnings	Passed with both dependency versions
Rust 1.88 with boring 4.19.0	All-targets check passed, 1,299 quiche lib tests and 45 doctests passed
Nightly fmt for the workspace, HTTP/3 tool and fuzz crate	Passed
Recovery and settings tests without unchecked congestion window	219 and 4 passed
C examples with -Werror, -pedantic and AddressSanitizer	All four built with explicit library paths and matching macOS framework and deployment flags
All-features documentation build	Built with 18 existing Rustdoc link warnings at unchanged baseline locations

The four original fault cases still fail on base 7055f78 under CUBIC and BBRv2 and pass with this change
Each of the three lifecycle mutations passes the original unit test and fails the expanded test

The new Stable, Nightly and Semgrep workflows require maintainer approval and have not executed any jobs on this head

[ghedo]

Thanks for addressing everything so quickly. This looks good to me, with two small test suggestions inline.