1034977 - Crash in mozalloc_abort(char const* const) | NS_DebugBreak | nsDebugImpl::Abort(char const*, int) with AsyncShutdownTimer "Sqlite.jsm Waiting for connections to close"

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 "Sqlite.jsm Waiting for connections to close"

As near as I can tell this has only been reported once so far:
https://crash-stats.mozilla.com/report/index/0e49ffb0-9749-4c93-82bc-8a89f2140627

Unfortunately, I don't know a good way to query crashstats to filter out only the crashes with this AsyncShutdownTimer message so I cannot be certain about the volume.

Comment 1

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

I suspect that this single crash was actually caused by a misbehaving add-on. Waiting for bug 1034726 and more signatures.

Comment 2

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

Actually, there are 133 of these crashes on Nightly, 138 on Aurora, none on Beta. We need to investigate quickly.

Comment 3

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

Attachment: More logging for Sqlite AsyncShutdown

Instrumenting the shutdown to find out what's wrong. It would be really good if we could get this fixed in Aurora before the uplift (next week, iirc).

Comment 4

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

Attachment: 1. Fixing fatal typo in Sqlite.jsm

I may have found the issue. This does not remove the need for the other patch, which should prove useful in case there is more than one issue or in case we have misbehaving clients, but I hope that this fixes the topcrasher in Aurora and Nightly.

Self-reviewing given that 1/ it's a typo fix; 2/ none of the other reviewers are available at the moment.

Comment 5

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

Try: https://tbpl.mozilla.org/?tree=Try&rev=4915e9fd0821

Comment 6

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

Attachment: 1. Fixing fatal typo in Sqlite.jsm, v2

Same typo, but fixing on the other side. Still only 5 chars, still self-reviewing.
Try: https://tbpl.mozilla.org/?tree=Try&rev=6e7abb2f9336

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8451603 [details] [diff] [review]
More logging for Sqlite AsyncShutdown

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

I didn't realize you could log extra state like this. That's pretty cool.

Also, I just glanced at AsyncShutdown.jsm for the first time. It's brilliant. I can't wait for us to apply the same concepts to startup as well. Lazy service initialization FTW!

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/9a75a4fe32ac
https://hg.mozilla.org/integration/fx-team/rev/7f9ae5ccdaef

Comment 9

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

Comment on attachment 8451700 [details] [diff] [review]
1. Fixing fatal typo in Sqlite.jsm, v2

Approval Request Comment
[Feature/regressing bug #]: bug 1028887
[User impact if declined]: unpredictable shutdown freeze/crashes
[Describe test coverage new/current, TBPL]: This is just a typo fix. Just landed on m-c.
[Risks and why]: None.
[String/UUID change made/needed]: None.

Comment 10

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

Comment on attachment 8451603 [details] [diff] [review]
More logging for Sqlite AsyncShutdown

Approval Request Comment
[Feature/regressing bug #]: Bug 1028887.
[User impact if declined]: This patch introduces logging that should help us fix unpredictable shutdown freeze/crash if the other patch is not sufficient.
[Describe test coverage new/current, TBPL]: Just landed on m-c.
[Risks and why]: This patch just adds logging.
[String/UUID change made/needed]: None.

Comment 11

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

(In reply to Gregory Szorc [:gps] from comment #7)
> Comment on attachment 8451603 [details] [diff] [review]
> More logging for Sqlite AsyncShutdown
> 
> Review of attachment 8451603 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I didn't realize you could log extra state like this. That's pretty cool.
> 
> Also, I just glanced at AsyncShutdown.jsm for the first time. It's
> brilliant. I can't wait for us to apply the same concepts to startup as
> well. Lazy service initialization FTW!

Thanks :)

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9a75a4fe32ac
https://hg.mozilla.org/mozilla-central/rev/7f9ae5ccdaef

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8451603 [details] [diff] [review]
More logging for Sqlite AsyncShutdown

We want logging. Approved!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Looks like the typo fix was for code that doesn't live on Aurora, so I didn't land that patch. The other one required some rebasing, so please take a look to make sure everything's OK with what landed.

Comment 15

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

This looks good, thanks.
Apparently, according to http://yoric.github.io/are-we-shutting-down-yet-/&sample_size=500#Sqlite.jsm%20shutdown%20blocker, we do not have recent occurrences of the bug. Marking as fixed.