1034975 - Crash in mozalloc_abort(char const* const) | NS_DebugBreak | nsDebugImpl::Abort(char const*, int) with AsyncShutdownTimer "Search service: shutting down"

Status: RESOLVED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[u279076]

See also bug 944873 comment 51.

1. Load https://crash-stats.mozilla.com/report/list?signature=mozalloc_abort%28char%20const*%20const%29%20|%20NS_DebugBreak%20|%20nsDebugImpl%3A%3AAbort%28char%20const*%2C%20int%29#tab-reports
2. Click "Change columns in this table" or scroll to the bottom of the page
3. Select "AsyncShutdownTimeout" from the "Available columns" list 
4. Click ">>" to add it to the list of "Chosen columns"
5. Click "Save and Reload"
6. Find in page "Search service: shutting down" and you'll see a lot of these crashes.

I don't know what the exact volume of this is as I don't know a good way to query crashstats on this AsyncShutdownTimer message. I only see one crash in the first page of the reports table though so this *may* be low volume.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Making Shutdown of Search Service depend on OS.File.profileBeforeChange rather than AsyncShutdown.profileBeforeChange.

Search Service is not registered as a client of OS.File shutdown although it depends on it. This is a possible cause of AsyncShutdown timeouts.

Fixing this and taking the opportunity to add more logging in case of crashes.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Try: https://tbpl.mozilla.org/?tree=Try&rev=9eaa1458b840

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8451369 [details] [diff] [review]
Making Shutdown of Search Service depend on OS.File.profileBeforeChange rather than AsyncShutdown.profileBeforeChange.

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

::: toolkit/components/search/nsSearchService.js
@@ +4198,5 @@
> +            }
> +
> +            // Ensure that error is reported and that it causes tests
> +            // to fail.
> +            Promise.reject(ex);

Don't you need to return this?

Comment 4

[David Teller [:Yoric] - still alive but not very active]

In this case, I'm using uncaught Promise.rejects as a super Cu.reportError that also causes test failures, so, I do not want this promise to be caught.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/0acf328ad8ed

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/0acf328ad8ed

Comment 7

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8451369 [details] [diff] [review]
Making Shutdown of Search Service depend on OS.File.profileBeforeChange rather than AsyncShutdown.profileBeforeChange.

Approval Request Comment
[Feature/regressing bug #]: Bug 940408
[User impact if declined]: Rare freeze/crashes during shutdown.
[Describe test coverage new/current, TBPL]: Just landed on mozilla-central. This probably deserves to cook a few days.
[Risks and why]: None that I can think of.
[String/UUID change made/needed]: None.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4c63c27ed22d