1234170 - crash in nsIWebSocketChannel::Serial

Status: RESOLVED FIXED

(Core :: Networking: WebSockets, defect)

Description

[[:philipp]]

[Tracking Requested - why for this release]:

This bug was filed from the Socorro interface and is 
report bp-961fe8ba-b63e-4212-b6af-e0e292151219.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	nsIWebSocketChannel::Serial() 	obj-firefox/dist/include/nsIWebSocketChannel.h
1 	xul.dll 	mozilla::dom::WebSocket::CreateAndDispatchCloseEvent(bool, unsigned short, nsAString_internal const&) 	dom/base/WebSocket.cpp
2 	xul.dll 	mozilla::dom::WebSocketImpl::DispatchConnectionCloseEvents() 	dom/base/WebSocket.cpp
3 	xul.dll 	mozilla::dom::CallDispatchConnectionCloseEvents::Run() 	dom/base/WebSocket.cpp
4 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
5 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp
6 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc
7 	xul.dll 	nsThreadManager::UnregisterCurrentThread(nsThread*) 	xpcom/threads/nsThreadManager.cpp
8 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp
9 	xul.dll 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp
10 	xul.dll 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp
11 	xul.dll 	XREMain::XRE_main(int, char** const, nsXREAppData const*) 	toolkit/xre/nsAppRunner.cpp
12 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp

requesting tracking since it is a regression in firefox 44 and taking the place as #12 most frequent crash in 44.0b1 data with ~1.2%.

Comment 1

[Andrew Overholt [:overholt]]

baku, please take a look.

Comment 2

[Andrea Marchesini [:baku]]

Attachment: ws.patch

This happens because of: https://mxr.mozilla.org/mozilla-central/source/dom/base/WebSocket.cpp#514

Comment 3

[Jason Duell]

Comment on attachment 8701152 [details] [diff] [review]
ws.patch

Review of attachment 8701152 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a DOM peer, but I did write the websocket early-failure code, and this looks right to me.  So hopefully it's OK for me to +r this.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

it is fine :)

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Remember to ask approvals to all the relevant branches.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe540291f72f

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 8701152 [details] [diff] [review]
ws.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1215092
[User impact if declined]: a crash
[Describe test coverage new/current, TreeHerder]: no test for it.
[Risks and why]: There are no risks here: I just add a null check before using mChannel.
[String/UUID change made/needed]: none

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fe540291f72f

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8701152 [details] [diff] [review]
ws.patch

One-liner not null crash fix, Beta44+, Aurora45+

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b151c99ab46a

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/ff3b505eabf0

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/ff3b505eabf0