Skip to content

fix(ttstream): ignore late header frame after stream close instead of tearing down mux transport - #2007

Open
linhongyu510 wants to merge 1 commit into
cloudwego:mainfrom
linhongyu510:fix/ttstream-late-header-after-close
Open

linhongyu510 wants to merge 1 commit into
cloudwego:mainfrom
linhongyu510:fix/ttstream-late-header-after-close

Conversation

@linhongyu510

Copy link
Copy Markdown

What type of PR is this?

  • Bug fix

What this PR does / why we need it:

After a client stream is close()d, an in-flight header frame can still be
dispatched to onReadHeaderFrame. This is a benign late frame, but today it
surfaces as a fatal errUnexpectedHeader (code 12002), which the read loop
treats as a protocol error and uses to tear down the whole mux transport
connection
(deferred Close(err)) — taking down every other stream sharing
the connection.

Root cause

close() parks a terminal signal (streamSigNone / streamSigInactive) into
the buffered headerSig channel (size 1) before marking the stream
inactive. A late header then:

  1. sets s.header = fr.header,
  2. tries to send streamSigActive into the now-full headerSig channel,
  3. hits the select default branch, which returned errUnexpectedHeader.

Fix

In the default branch, when the stream is already streamStateInactive, the
header is an in-flight frame for an already-closed stream: drop it (s.header = nil) and return nil so the mux transport survives.

A genuine duplicate header (two header frames, no close in between) still
errors: it either hits the s.header != nil guard, or reaches the default
branch on an active stream.

Which issue(s) this PR fixes:

N/A (found via race-oracle testing; no linked issue).

Special notes for your reviewer:

  • The added check is an atomic.LoadInt32(&s.state) == streamStateInactive,
    matching the existing state-machine convention in this file.
  • Regression tests added in stream_client_lateheader_test.go:
    • Test_onReadHeaderFrameAfterCloseIsIgnored — late header after close is
      ignored, s.header not leaked.
    • Test_onReadDuplicateHeaderStillErrors — true duplicate still returns
      errUnexpectedHeader (guards against over-loosening).
    • Test_onReadHeaderFrameRaceWithClose — races close() against
      onReadHeaderFrame for 200 iterations; never produces a fatal error.
  • Verified: on unmodified main, the late-header and race tests fail with
    12002 and the duplicate test still passes; with the fix all pass under
    -race. go vet clean; cross-compiles for linux/amd64, linux/arm64,
    darwin/{arm64,amd64}, windows/amd64.

close() parks a terminal signal (streamSigNone/streamSigInactive) in the
buffered headerSig channel before marking the stream inactive. When an
in-flight header frame is dispatched afterwards, onReadHeaderFrame fills
s.header but fails to send streamSigActive into the now-full channel and
hits the select default branch, which returned errUnexpectedHeader. The
read loop treats that protocol error as fatal and tears down the whole mux
transport connection (deferred Close(err)), taking down all other streams
on the connection.

In the default branch, if the stream is already inactive, the header is
an in-flight frame for an already-closed stream: drop it and return nil so
the transport survives. A genuine duplicate header (no close in between)
is still rejected, since it either hits the s.header!=nil guard or reaches
the default branch on an active stream.
@linhongyu510
linhongyu510 requested review from a team as code owners October 10, 2026 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant