1653642 - Crash in [@ mozilla::net::CallOnStop::~CallOnStop]

Status: RESOLVED FIXED

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

Description

[u608768]

This bug is for crash report bp-61a11c5f-92cc-4df1-bba3-647150200717.

Top 10 frames of crashing thread:

0 libxul.so mozilla::net::CallOnStop::~CallOnStop netwerk/protocol/websocket/WebSocketChannel.cpp:593
1 libxul.so mozilla::net::CallOnStop::~CallOnStop netwerk/protocol/websocket/WebSocketChannel.cpp:593
2 libxul.so mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal xpcom/string/nsSubstring.cpp
3 libxul.so mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal xpcom/threads/nsThreadUtils.h:577
4 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1234
5 libxul.so nsThread::Shutdown xpcom/threads/nsThread.cpp:898
6 libxul.so mozilla::net::nsSocketTransportService::ShutdownThread netwerk/base/nsSocketTransportService2.cpp:853
7 libxul.so mozilla::net::nsIOService::SetOffline netwerk/base/nsIOService.cpp:1314
8 libxul.so mozilla::net::nsIOService::Observe netwerk/base/nsIOService.cpp:1601
9 libxul.so non-virtual thunk to mozilla::net::nsIOService::Observe netwerk/base/nsIOService.cpp

Started in build 20200716212737. Regression window: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c8e9ecbc6236ea4cf1aa5063f20583e1f574f2b9&tochange=a35461d1fc07e6e855d064453363a46d32bb5a3d

kershaw, could this have been bug 1497249.

Comment 1

[Kershaw Chang [:kershaw]]

Based on this report, It seems that the WebSocketChannel is already released. Since the channel is stored by a strong pointer in CallOnStop, the address should be already invalid when constructing CallOnStop. I think this should be caused by releasing the channel when nsWebSocketConnection::Close() is called here.

Comment 2

[Kershaw Chang [:kershaw]]

Attachment: Bug 1653642 - Don't release WebSocketChannel in nsWebSocketConnection::Close, r=michal

Comment 3

[Michal Novotny [:michal]]

(In reply to Kershaw Chang [:kershaw] from comment #1)

the address should be already invalid when constructing CallOnStop.

But what scenario could lead to calling WebSocketChannel::DoStopSession() on a freed object? I would expect there is somewhere a strong reference on the callstack...

Comment 4

[Kershaw Chang [:kershaw]]

(In reply to Michal Novotny [:michal] from comment #3)

(In reply to Kershaw Chang [:kershaw] from comment #1)

the address should be already invalid when constructing CallOnStop.

But what scenario could lead to calling WebSocketChannel::DoStopSession() on a freed object? I would expect there is somewhere a strong reference on the callstack...

I think (In reply to Michal Novotny [:michal] from comment #3)

(In reply to Kershaw Chang [:kershaw] from comment #1)

the address should be already invalid when constructing CallOnStop.

But what scenario could lead to calling WebSocketChannel::DoStopSession() on a freed object? I would expect there is somewhere a strong reference on the callstack...

I think this happens when WebSocketChannel::CleanupConnection() is called inside WebSocketChannel::DoStopSession(). Could be here. I assume at this point, the only strong reference that holds WebSocketChannel is the mConnection. And then, nsWebSocketConnection::Close() is called and the channel is released at here. At the end, an invalid address is passed to CallOnStop's constructor.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ab7f1314221
Don't release WebSocketChannel in nsWebSocketConnection::Close, r=michal,necko-reviewers

Comment 6

[Narcis Beleuzu [:NarcisB]]

Backed out for remote failures on nsWebSocketConnection.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/9a37a0e7de35daa6572a1499556a9e814cff7bc8
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310416766&repo=autoland&lineNumber=1689

Comment 7

[Kershaw Chang [:kershaw]]

(In reply to Narcis Beleuzu [:NarcisB] from comment #6)

Backed out for remote failures on nsWebSocketConnection.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/9a37a0e7de35daa6572a1499556a9e814cff7bc8
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310416766&repo=autoland&lineNumber=1689

https://treeherder.mozilla.org/#/jobs?repo=try&revision=78c14e3fb0f6148b07178d86ba5117d5a948546a

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c9946becbca
Don't release WebSocketChannel in nsWebSocketConnection::Close, r=michal,necko-reviewers

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1c9946becbca

Comment 11

[Alexandru Michis [:malexandru]]

Backed out 3 changesets (bug 1653642, bug 1497249) for causing Bug 1654282.
Backed out changeset 1c9946becbca.
Link: https://hg.mozilla.org/integration/autoland/rev/d72591a16d42a6cefcffe3df901fa9d0db440193

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/d72591a16d42

Comment 13

[Kershaw Chang [:kershaw]]

This should be fixed by backing out bug 1497249.