1720079 - All loading gets stuck after attempting to upload a video to subrock.rocks with HTTP3

Status: VERIFIED FIXED

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

Description

[Kestrel]

Attachment: httplog.txt

STR:

1. Make sure network.http.http3.enabled = true and dom.security.https_only_mode = true.
2. Create an account at https://subrock.rocks (no email verification required).
3. Attempt to upload a video file larger than the maximum size (100MB).

Before upload commences, the site shows an alert dialog:

Fatal error or stopped uploading. Try again.

After pressing OK, the tab shows a loading indicator but page gets stuck, cannot refresh or navigate away. 100% CPU core for main process. Other tabs won't load or navigate. Closing browser triggers crash.

Network log is filled with:

V/nsHttp Http3Session::ProcessEvents - HeaderReady
V/nsHttp Http3Session::ProcessEvents - HeaderReady - stream not found stream_id=0x0

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

We're going into an infinite loop here: https://searchfox.org/mozilla-central/rev/b79212b4fc017f27ac2435a658d4e9b9798efa86/netwerk/protocol/http/Http3Session.cpp#376

We are continuing the loop without getting the next event. We should either break out of the switch block, which will get the next event and then continue the loop, or return an error. I think the former is appropriate, the other block that null-checks the stream also silently continues if the stream was not found.

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1720079 - Don't advance Http3Session's ProcessEvents loop without getting the next event. r=#necko!

Comment 3

[Julien Cristau [:jcristau]]

How common is this, and do we need to uplift?

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

(In reply to Julien Cristau [:jcristau] from comment #3)

How common is this, and do we need to uplift?

I'm not sure how common this is, but uplift is probably the way to go considering the simplicity of the patch and impact of the bug when it manifests. I'll request after this makes it into m-c successfully.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/037268a42cf7
Don't advance Http3Session's ProcessEvents loop without getting the next event. r=necko-reviewers,kershaw

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/037268a42cf7

Comment 7

[Pascal Chevrel:pascalc]

Nihanth, can you request uplift today so as to have it in beta 3? Thanks!

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 9230883 [details]
Bug 1720079 - Don't advance Http3Session's ProcessEvents loop without getting the next event. r=#necko!

Beta/Release Uplift Approval Request

• User impact if declined: This bug causes high cpu usage and DoS's the ability to browse the web when triggered. At the moment it's unclear how often it might be manifesting in the wild, but since the server can trigger this it's... not ideal.

I've asked for approval-mozilla-release opportunistically - I don't think this is worth a dot release but if we do one it would be nice to sneak this in.

• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: In comment 0. I verified this myself in latest Nightly.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch itself is simple and an obvious correction.
• String changes made/needed: none

Comment 9

[Pascal Chevrel:pascalc]

Comment on attachment 9230883 [details]
Bug 1720079 - Don't advance Http3Session's ProcessEvents loop without getting the next event. r=#necko!

Approved for 91 beta 3, thanks.

Comment 10

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/7f5d88c56067

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9230883 [details]
Bug 1720079 - Don't advance Http3Session's ProcessEvents loop without getting the next event. r=#necko!

approved for 90.0.1, thanks

Comment 12

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/967b7ad9a5f6

Comment 13

[Daniel Bodea [:danibodea]]

The behavior I've seen in Firefox Release v89.0 (affected):
Actual Result: After pressing OK, the tab shows a loading indicator but the page gets stuck, cannot refresh, or navigate away. did NOT see 100% CPU core for the main process, as reported originally. Other tabs won't load or navigate. Closing browser DOES NOT trigger a crash, as originally reported, but Firefox processes are left hanging and the crash happens when reopening the browser:
https://crash-stats.mozilla.org/report/index/5eece38e-42e3-4065-b1b7-7e2f30210719

I've updated to the latest version that contains the fix, Fx 90.0 (RC).
The new result is pretty much the same, but has a different crash report:
https://crash-stats.mozilla.org/report/index/b703eb72-541b-4189-b35a-bb58d0210719#tab-details

The behavior I've seen in Firefox Beta v90.0b12 (affected): most of the issue reproduces, the freeze is seen, processes are left hanging, but no crash is observed.

After an update to v91.0b4 (fixed), the browser does not freeze, nor does it leave processes after closing it or show crashes. I deem this version verified.

The behavior I've seen in Firefox Nightly v92.0a1 (affected): freeze, hanging processes, crash at restart:
https://crash-stats.mozilla.org/report/index/213839a0-b2ec-482f-b6ed-6ceaa0210719#tab-details

After an update to Nightly fixed version: the browser does not freeze, nor does it leave processes after closing it or show crashes. I deem this version verified.

After checking the push logs, it appears that the fix has not reached the latest version of the release channel, RC 90.0. Cannot verify Fx90, yet, but the others are verified.

Comment 14

[Daniel Bodea [:danibodea]]

I've tested Firefox Release "dot candidate" 90.0.1, build ID 20210716144314 and the fix is verified. Thank you.

Comment 15

[Julien Cristau [:jcristau]]

Added to 90.0.1 release notes: "Fixed busy looping processing some HTTP3 responses"