1597915 - Crash in [@ AsyncShutdownTimeout | Places Clients shutdown | sanitize.js: Sanitize]

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Pascal Chevrel:pascalc]

This bug is for crash report bp-b2397b60-80e8-4146-a7d0-a9cfb0191120.

Top 10 frames of crashing thread:

0 mozglue.dll mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:33
1 xul.dll static void Abort xpcom/base/nsDebugImpl.cpp:442
2 xul.dll NS_DebugBreak xpcom/base/nsDebugImpl.cpp
3 xul.dll nsresult nsDebugImpl::Abort xpcom/base/nsDebugImpl.cpp:134
4 xul.dll XPTC__InvokebyIndex 
5  @0x1ef598c1a2f 
6 xul.dll trunc 
7 xul.dll trunc 
8 xul.dll static bool XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1149
9 xul.dll static bool XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:946

High volume on Nightly the last days

Comment 1

[Julien Cristau [:jcristau]]

Pushlog is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d6844fe546ad&tochange=f78494c785f1b3264b1a17376d01520e1072ba7b

Comment 2

[Julien Cristau [:jcristau]]

Let's keep this about the recent regression for this signature in nightly.

Comment 3

[Andrew McCreight [:mccr8]]

Ehsan, would you mind taking a look at this Nightly regression? Your patch for bug 1595934 (or maybe some of the other things you landed?) is the most obviously session store related thing in the change sets in comment 1. Do you think it could be related?

Comment 4

[(no longer active)]

(In reply to Andrew McCreight [:mccr8] from comment #3)

Ehsan, would you mind taking a look at this Nightly regression? Your patch for bug 1595934 (or maybe some of the other things you landed?) is the most obviously session store related thing in the change sets in comment 1. Do you think it could be related?

I didn't land anything else that could have caused this in that range (bug 1596843 was backed out, and bug 1595935 removes some expired telemetry probes which would be reported at startup.)

Neither the crash report nor its relationship to bug 1595934 make sense to me, but there is only one way to be sure. I asked the sheriff to back out that bug and we'll see. :-)

Comment 5

[Marco Bonardo [:mak]]

Apart from the obvious Places string in the name, I don't think this has anything to do with Places, it just happens that the sanitizer is blocking Places shutdown (because we must wait for removals before closing the connection), but the problem is in some consumers of the Sanitizer.

I'm looking again at aggregations based on Async Shutdown Timeout, those contain useful metadata to see where we block.
Tentatively I'm moving this to cookies since it's clear there's a large amount of blocking there.

AsyncShutdownTimeout | Places Clients shutdown | sanitize.js: Sanitize

• 12.72% crash with {"cache":"cleared","cookies":"cleared","history":"cleared","formdata":"blocking","downloads":"cleared","sessions":"cleared"}}
  => FormData
• 9.66% crash with {"cache":"cleared","cookies":"blocking","history":"cleared","formdata":"cleared","downloads":"cleared","sessions":"cleared"}}
  => Cookies
• 4% crash with {"cache":"cleared","cookies":"blocking","history":"cleared","formdata":"cleared","downloads":"cleared","sessions":"cleared"}}
  => Cookies
• 3.65% crash with {"cache":"cleared","cookies":"blocking","history":"cleared","formdata":"cleared","downloads":"cleared","sessions":"cleared"}}
  => Cookies
• ...

AsyncShutdownTimeout | Places Clients shutdown | sanitize.js: Sanitize,sanitize.js: Sanitize

• 10% crash with {"cache":"cleared","cookies":"blocking","history":"cleared","formdata":"cleared","downloads":"cleared","sessions":"cleared"}}
  => Cookies
• 6.25% + 6.25% crash with {"cache":"cleared","cookies":"blocking","offlineApps":"cleared","history":"cleared","formdata":"cleared","downloads":"cleared","sessions":"cleared","siteSettings":"cleared"}
  => Cookies
• 3.75% crash with {"cache":"cleared","cookies":"cleared","history":"cleared","formdata":"blocking","downloads":"cleared","sessions":"cleared"}}
  => FormData

Matt, there is also some percentage blocking on FormData, any idea?

Comment 6

[(no longer active)]

BTW bug 1595934 was backed out yesterday to see if it impacts the crash rates here.

Comment 7

[(no longer active)]

Do I understand correctly that the crash rates have dropped after bug 1595934 being backed out? Thanks!

Comment 8

[Julien Cristau [:jcristau]]

Yes, looks that way. Builds starting with 20191122091512 have few or no crashes, and that's the first build with https://hg.mozilla.org/mozilla-central/rev/adf7d49a44f8

Comment 9

[Matthew N. [:MattN]]

I'm not sure what information you want from me but there haven't been any recent changes to form history storage. Is this the same stack that would be used from the web extension API to clear this data? If so, I wonder if there could be issues of contention between multiple extensions clearing the data at shutdown.

Comment 10

[Julien Cristau [:jcristau]]

This does look fixed by the backout.

Comment 11

[(no longer active)]

I've relanded bug 1595934 about three days ago, after landing bug 1599237 which was my guess fix for this crash, and looking at crash-stats it seems like that fix worked and we're not seeing this crash spike up again.

Please reopen this bug in case I'm misreading crash-stats, thanks. Clearing my needinfo now since I think my work here is done.