Firefox crashes in [@ mozilla::dom::WebSocket::CreateAndDispatchCloseEvent ]

VERIFIED FIXED in Firefox 52

Status

()

--
critical
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: Virtual, Assigned: amarchesini)

Tracking

({crash, crashreportid, nightly-community})

53 Branch
mozilla53
crash, crashreportid, nightly-community
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr45 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 verified, firefox53 verified)

Details

(crash signature)

Attachments

(1 attachment)

I was opening about 50-100 tabs in one go and Firefox tabs crashes in [@ mozilla::dom::WebSocket::CreateAndDispatchCloseEvent ]

Crashlog report:
https://crash-stats.mozilla.com/report/index/55d75fde-fb4e-4d64-99be-fb5212161214
Michal, can you take a look at this when you get a chance? (I seem to remember you being Mr. WebSockets)
Flags: needinfo?(michal.novotny)
Whiteboard: [necko-next]
OS: Windows 7 → All
Hardware: x86_64 → All
It seems this has nothing to with the WebSocketChannel living in necko. I guess WebSocketImpl::Disconnect() which nulled out mImpl member was called before WebSocket::CreateAndDispatchCloseEvent(), but I don't know the code at all. Andrea, can you have a look at it?
Component: Networking: WebSockets → DOM
Flags: needinfo?(michal.novotny) → needinfo?(amarchesini)
Whiteboard: [necko-next]
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Created attachment 8824024 [details] [diff] [review]
ws.patch
Attachment #8824024 - Flags: review?(bugs)

Updated

2 years ago
Attachment #8824024 - Flags: review?(bugs) → review+

Comment 4

2 years ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38b4fc72c168
WebSocket::CreateAndDispatchCloseEvent must check if mImpl has been nullified, r=smaug

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/38b4fc72c168
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Has Regression Range: --- → irrelevant
Has STR: --- → yes
Status: RESOLVED → VERIFIED
status-firefox53: fixed → verified
Looks like a low-risk patch? Please nominate for Aurora uplift.
status-firefox50: --- → wontfix
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox-esr45: --- → wontfix
Flags: needinfo?(amarchesini)
Comment on attachment 8824024 [details] [diff] [review]
ws.patch

Approval Request Comment
[Feature/Bug causing the regression]: WebSockets in workers
[User impact if declined]: a crash can occur.
[Is this code covered by automated tests?]: no, it's racy.
[Has the fix been verified in Nightly?]: yes.
[Needs manual test from QE? If yes, steps to reproduce]: yes, see the description of the bug. 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: It's just the use of a nullified pointer.
[String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8824024 - Flags: approval-mozilla-aurora?
Attachment #8824024 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8824024 [details] [diff] [review]
ws.patch

crash fix for beta52, should be in 52.0b2
Attachment #8824024 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 9

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/b01263e0808e
status-firefox52: affected → fixed
Flags: qe-verify+
I tried to reproduce this crash multiple times but without success (windows 7 x86 using Nightly from 2016-12-30). Reporter would you be able to verify if you get this crash on latest Firefox 52 beta build? Thanks!

https://archive.mozilla.org/pub/firefox/candidates/52.0b4-candidates/build1/
Flags: needinfo?(virtual)
It's kinda very hard to reproduce, as only less than 5% tried will produce the crash), but in the end the bug looks fixed also on 52, because I didn't encounter any crash for a whole day and crash statistics looks clean.
status-firefox52: fixed → verified
Flags: needinfo?(virtual)
You need to log in before you can comment on or make changes to this bug.