2030336 - HappyEyeballsConnectionAttempt::Claim() can strand transactions when another attempt wins

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Kershaw Chang [:kershaw]]

When HappyEyeballsConnectionAttempt::Claim() replaces the NullTransaction with a real nsHttpTransaction, the transaction is removed from the pending queue. If another HE attempt for the same ConnectionEntry wins the connection race (e.g., via H2 coalescing triggering MakeAllDontReuseExcept -> CloseAllConnectionAttempts), the losing attempt is abandoned.
The claimed transaction is never closed or requeued.

Comment 1

[Kershaw Chang [:kershaw]]

Attachment: Bug 2030336 - Don't hand over the real transaction in HappyEyeballsConnectionAttempt::Claim, r=mxinden

Keep the same behavior as DnsAndConnectSocket::Claim: reset speculative flags, but leave the real transaction in the pending queue. This avoids orphaning the transaction when another happy eyeballs attempt wins the connection race and the losing attempt is abandoned.

Update test_happy_eyeballs_timings.js, since the real transaction no longer observes connection-phase transport statuses from the speculative phase.

Comment 2

[Kershaw Chang [:kershaw]]

Attachment: Bug 2030336 - Only check CanReuse for H2/H3 in ConnectionEstablisher::FinishInternal, r=mxinden

The previous fix for the crash in Http3Session::AddStream is not completely correct.
For H1/H2, we should skip the check so the transaction receives the connection and gets the proper error code (e.g., certificate verification failure). CanReuse on fresh H1 connections can also return false negatives when mKeepAlive is unset.
For H3, we should keep the CanReuse check to guard against sessions being destroyed between the async handshake callback and FinishInternal.

Comment 3

[Kershaw Chang [:kershaw]]

Attachment: Bug 2030336 - Requeue claimed transactions when H2 coalescing abandons HE attempts, r=mxinden

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/37993a983e98
https://hg.mozilla.org/integration/autoland/rev/bc8fa0ed1110
Only check CanReuse for H2/H3 in ConnectionEstablisher::FinishInternal, r=mxinden
https://github.com/mozilla-firefox/firefox/commit/7a890251695e
https://hg.mozilla.org/integration/autoland/rev/9334bd54589e
Requeue claimed transactions when H2 coalescing abandons HE attempts, r=mxinden,necko-reviewers,valentin

Comment 5

[agoloman]

Backed out for causing wpt failures @binaryType-wrong-value.any.html.

• Backout link
• Push with failures
• Failure Log

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/03274a4ed83c
https://hg.mozilla.org/integration/autoland/rev/19894daef11b
Only check CanReuse for H2/H3 in ConnectionEstablisher::FinishInternal, r=mxinden
https://github.com/mozilla-firefox/firefox/commit/85652ba50ae3
https://hg.mozilla.org/integration/autoland/rev/c6105b7a5c00
Requeue claimed transactions when H2 coalescing abandons HE attempts, r=mxinden,necko-reviewers,valentin

Comment 7

[rperta]

https://hg.mozilla.org/mozilla-central/rev/bc8fa0ed1110
https://hg.mozilla.org/mozilla-central/rev/9334bd54589e
https://hg.mozilla.org/mozilla-central/rev/19894daef11b
https://hg.mozilla.org/mozilla-central/rev/c6105b7a5c00