1806332 - [diagnostic assert] Crash in [@ mozilla::net::CacheOutputCloseListener::OnOutputClosed]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

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

5 such crashes from 5 different machines for a diagnostic assert added in bug 1799222.

Crash report: https://crash-stats.mozilla.org/report/index/f7009a6e-e63c-4d9c-a709-0e8a30221218

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (NS_SUCCEEDED(sts->Dispatch(do_AddRef(this))))

Top 10 frames of crashing thread:

0  xul.dll  mozilla::net::CacheOutputCloseListener::OnOutputClosed  netwerk/cache2/CacheEntry.cpp:1894
0  xul.dll  mozilla::net::CacheFileOutputStream::NotifyCloseListener  netwerk/cache2/CacheFileOutputStream.cpp:349
0  xul.dll  mozilla::net::CacheFile::RemoveOutput  netwerk/cache2/CacheFile.cpp:2153
1  xul.dll  mozilla::net::CacheFileOutputStream::Release  netwerk/cache2/CacheFileOutputStream.cpp:28
2  xul.dll  nsCOMPtr_base::assign_assuming_AddRef  xpcom/base/nsCOMPtr.h:377
2  xul.dll  nsCOMPtr<nsIOutputStream>::assign_assuming_AddRef  xpcom/base/nsCOMPtr.h:400
2  xul.dll  nsCOMPtr<nsIOutputStream>::operator=  xpcom/base/nsCOMPtr.h:696
2  xul.dll  mozilla::net::nsStreamListenerTee::OnStopRequest  netwerk/base/nsStreamListenerTee.cpp:47
3  xul.dll  mozilla::net::nsHttpChannel::ContinueOnStopRequest  netwerk/protocol/http/nsHttpChannel.cpp:7718
4  xul.dll  mozilla::net::nsHttpChannel::OnStopRequest  netwerk/protocol/http/nsHttpChannel.cpp:7410

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1806332 - Dispatch CacheOutputCloseListener to main thread when already there, r=kershaw!

This should avoid the crash caused by the CacheOutputCloseListener being
fired on the main thread too late during shutdown by keeping the
previous behaviour when on the main thread.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0030692eb1ae
Dispatch CacheOutputCloseListener to main thread when already there, r=kershaw,necko-reviewers

Comment 3

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/0030692eb1ae

Comment 4

[Kai Engert [:KaiE:]]

Nika, why is it necessary to call NS_DispatchToMainThread, when you're already on the main thread?
Why isn't it sufficient to directly perform the necessary action?

I'm asking because we're seeing a crash in bug 1806989 in this call to NS_DispatchToMainThread on a macos debug build.
(We can discuss other details of the crash over in bug 1806989, but I thought it might be helpful to ask this general question here.)

Comment 5

[Kai Engert [:KaiE:]]

(In reply to Kai Engert (:KaiE:) from comment #4)

Nika, why is it necessary to call NS_DispatchToMainThread, when you're already on the main thread?
Why isn't it sufficient to directly perform the necessary action?

Ok, sorry, there's a comment that explains it:
https://searchfox.org/mozilla-central/rev/55d5c4b9dffe5e59eb6b019c1a930ec9ada47e10/netwerk/cache2/CacheEntry.cpp#1887

// We need this class and to redispatch since this callback is invoked
// under the file's lock and to do the job we need to enter the entry's
// lock too. That would lead to potential deadlocks.

Comment 6

[Nika Layzell [:nika] (ni? for response)]

(In reply to Kai Engert (:KaiE:) from comment #5)

Ok, sorry, there's a comment that explains it:
https://searchfox.org/mozilla-central/rev/55d5c4b9dffe5e59eb6b019c1a930ec9ada47e10/netwerk/cache2/CacheEntry.cpp#1887

Yup, that's why :-)