1603684 - Crash in [@ mozilla::dom::ServiceWorkerRegistrar::GetShutdownPhase]

Status: RESOLVED DUPLICATE of bug 1403348

(Core :: DOM: Service Workers, defect, P2)

Description

[Pascal Chevrel:pascalc]

This bug is for crash report bp-2f201de4-c9fe-4f41-9123-2c7c00191213.

Top 10 frames of crashing thread:

0 xul.dll class nsCOMPtr<nsIAsyncShutdownClient> mozilla::dom::ServiceWorkerRegistrar::GetShutdownPhase dom/serviceworkers/ServiceWorkerRegistrar.cpp
1 xul.dll void mozilla::dom::ServiceWorkerRegistrar::ProfileStarted dom/serviceworkers/ServiceWorkerRegistrar.cpp:1110
2 xul.dll nsresult mozilla::dom::ServiceWorkerRegistrar::Observe dom/serviceworkers/ServiceWorkerRegistrar.cpp:1253
3 xul.dll nsObserverList::NotifyObservers xpcom/ds/nsObserverList.cpp:66
4 xul.dll nsObserverService::NotifyObservers xpcom/ds/nsObserverService.cpp:291
5 xul.dll nsXREDirProvider::DoStartup toolkit/xre/nsXREDirProvider.cpp:968
6 xul.dll nsresult XREMain::XRE_mainRun toolkit/xre/nsAppRunner.cpp:4420
7 xul.dll int XREMain::XRE_main toolkit/xre/nsAppRunner.cpp:4746
8 xul.dll XRE_main toolkit/xre/nsAppRunner.cpp:4827
9 xul.dll int mozilla::BootstrapImpl::XRE_main toolkit/xre/Bootstrap.cpp:45

Philipp identified a spike of this signature since yesterday, most of the comments are in French

Comment 1

[Julien Cristau [:jcristau] (back April 22)]

crash reason is "Failed to get async shutdown service: NS_ERROR_FILE_NOT_FOUND"

Comment 2

[Julien Cristau [:jcristau] (back April 22)]

and the spike is specifically on 68esr

Comment 3

[Tom Tung [:tt, :ttung]]

The crash rate is rather high, but it seems there is no crash on Nightly. Maybe Perry or Andrew have an idea about what's going on?

Comment 4

[Simon Giesecke [:sg] [he/him]]

There are crashes on nightly, but the table does not show them for some reason. If you select nightly in the box under the graph, you will see them. I already noticed this on some other bug with crash reports. Maybe the number of rows is limited, and nightly is left out if there are too many other rows.

Comment 5

[Calixte Denizet (:calixte)]

The last crashes we had were in nightly 72.0a1 (buildid 20191201093732):
https://crash-stats.mozilla.org/search/?release_channel=nightly&signature=%3Dmozilla%3A%3Adom%3A%3AServiceWorkerRegistrar%3A%3AGetShutdownPhase&product=Firefox&date=%3E%3D2019-06-13T14%3A33%3A00.000Z&date=%3C2019-12-13T14%3A33%3A00.000Z&_facets=version&_facets=build_id&_facets=signature&_sort=-date&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-version

And the crash table shows only the data for nightly with buildid >= 20191206214833.

Comment 6

[Perry Jiang [:perry] [no longer employee, use ni?]]

I'm not sure what's going on. From the crash reports' MOZ_CRASH Reason (Sanitized): Failed to get profileBeforeChange shutdown blocker: NS_ERROR_FILE_NOT_FOUND, it appears that rv = svc->GetProfileBeforeChange(getter_AddRefs(client)); is failing. That method is implemented in JS, and I'm not sure where the NS_ERROR_FILE_NOT_FOUND error code is coming from...

Comment 7

[Tom Tung [:tt, :ttung]]

Hmm, I thought it's this line because the crash reason on the reports is "Failed to get async shutdown service: NS_ERROR_FILE_NOT_FOUND"

Checking the code for getting the async shutdown service, the other component usensCOMPtr<nsIAsyncShutdownService> svc = services::GetAsyncShutdown(); rather than nsCOMPtr<nsIAsyncShutdownService> svc = do_GetService("@mozilla.org/async-shutdown-service;1", &rv);, maybe that block of code in the ServiceWorkerRegistrar.cpp needs to be changed? However, I have no idea whether this is the cause, TBH.

Comment 8

[Jens Stutte [:jstutte]]

(In reply to Tom Tung [:tt, :ttung] from comment #7)

Hmm, I thought it's this line because the crash reason on the reports is "Failed to get async shutdown service: NS_ERROR_FILE_NOT_FOUND"

Yes, that seems to be the point of failure.

Checking the code for getting the async shutdown service, the other component usensCOMPtr<nsIAsyncShutdownService> svc = services::GetAsyncShutdown(); rather than nsCOMPtr<nsIAsyncShutdownService> svc = do_GetService("@mozilla.org/async-shutdown-service;1", &rv);, maybe that block of code in the ServiceWorkerRegistrar.cpp needs to be changed? However, I have no idea whether this is the cause, TBH.

mozilla::services::GetAsyncShutdown() calls an internal function that seems to do essentially the same but adds caching of the result. see 1. So I would not expect this change to help, though it is surely better to use the given functions and to not rely on the string identifiers copied into the code.

I see a thread working while this happens, Thread 24, Name: Cookie, it is deleting the cookie database ? Might be totally unrelated, but could this indicate, that something else did not yet initialize when we expect it to be ready? After all, it is async and we do not wait here... I did not yet check other traces, if this is a pattern, however.

Comment 9

[Tom Tung [:tt, :ttung]]

(In reply to Jens Stutte [:jstutte] from comment #8)

I see a thread working while this happens, Thread 24, Name: Cookie, it is deleting the cookie database ? Might be totally unrelated, but could this indicate, that something else did not yet initialize when we expect it to be ready? After all, it is async and we do not wait here... I did not yet check other traces, if this is a pattern, however.

If the user clicks the option "Delete cookies and site data when Nightly is closed" on about:preference, then it can happen that the cookie database is deleting cookie on this stage (profile-change-teardown) during shutting down.

Comment 10

[Jens Stutte [:jstutte]]

(In reply to Tom Tung [:tt, :ttung] from comment #9)

If the user clicks the option "Delete cookies and site data when Nightly is closed" on about:preference, then it can happen that the cookie database is deleting cookie on this stage (profile-change-teardown) during shutting down.

Oh, we are shutting down? I was misled probably by the startup crash flag set on most of these crashes. Or do we have an underlying problem of closing down immediately after startup for whatever bad reason, and this provokes an unexpected order of things? Anyway, the shutdown service clearly indicates what its name says.
So the shutdown probably provokes a premature unregister of "@mozilla.org/async-shutdown-service;1" such that we do not find it any more. Does this mean, that the underlying object has been destroyed? Because a call to mozilla::services::GetAsyncShutdown() would probably still return the cached pointer, at least I do not see any clearing of the gAsyncShutdown cache variable at a first glance. So if the service object has a longer lifetime than its registration, the change to mozilla::services::GetAsyncShutdown() could help (if this is not the first call) - otherwise it might cause a nasty UAF and we should solve the lifecycle problem or handle this situation more gracefully than with a crash.

Comment 11

[Tom Tung [:tt, :ttung]]

(In reply to Jens Stutte [:jstutte] from comment #10)

Oh, we are shutting down? I was misled probably by the startup crash flag set on most of these crashes. Or do we have an underlying problem of closing down immediately after startup for whatever bad reason, and this provokes an unexpected order of things? Anyway, the shutdown service clearly indicates what its name says.

Ah, sorry, it's startup. I overlooked it's "profile-after-change" and it's ProfileStarted. Please ignore my comment in comment #9

Comment 12

[Jens Stutte [:jstutte]]

Hmm. that would mean we are expecting a service to be registered during startup which is not yet ready? Then the change to mozilla::services::GetAsyncShutdown() will not make any difference, IMHO (but is still desirable). Can we wait for a service (or declare otherwise the dependency)?

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

I think this is the stalled bug 1403348 where the problem is that the JS component is failing to load, hence the NS_ERROR_FILE_NOT_FOUND.