Closed Bug 1597915 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Networking: Cookies, defect)

Unspecified
Windows 10
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- unaffected
firefox72 + fixed

People

(Reporter: pascalc, Unassigned)

References

Details

(Keywords: crash, regression)

Crash Data

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

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

Crash Signature: [@ AsyncShutdownTimeout | Places Clients shutdown | sanitize.js: Sanitize] → [@ AsyncShutdownTimeout | Places Clients shutdown | sanitize.js: Sanitize] [@ AsyncShutdownTimeout | Places Clients shutdown | sanitize.js: Sanitize,sanitize.js: Sanitize]

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?

Flags: needinfo?(ehsan)
Component: Storage: Quota Manager → Places
Product: Core → Toolkit

(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. :-)

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?

Component: Places → Networking: Cookies
Flags: needinfo?(MattN+bmo)
Product: Toolkit → Core

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

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

Flags: needinfo?(ehsan)
Flags: needinfo?(ehsan)

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

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.

Flags: needinfo?(MattN+bmo)

This does look fixed by the backout.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

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.

Flags: needinfo?(ehsan)
Blocks: 1599237
You need to log in before you can comment on or make changes to this bug.