1472573 - Crash in mozilla::dom::ServiceWorkerRegistrationInfo::ServiceWorkerRegistrationInfo

Status: RESOLVED INCOMPLETE

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-e926174a-934b-4fdf-bbf0-d5f0c0180701.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::ServiceWorkerRegistrationInfo::ServiceWorkerRegistrationInfo dom/serviceworkers/ServiceWorkerRegistrationInfo.cpp:110
1 xul.dll mozilla::dom::ServiceWorkerManager::LoadRegistration dom/serviceworkers/ServiceWorkerManager.cpp:1527
2 xul.dll mozilla::dom::ServiceWorkerManager::Init dom/serviceworkers/ServiceWorkerManager.cpp:287
3 xul.dll mozilla::dom::ServiceWorkerManager::GetInstance dom/serviceworkers/ServiceWorkerManager.cpp:1420
4 xul.dll mozilla::image::ImageCacheKey::GetSpecialCaseDocumentToken image/ImageCacheKey.cpp:134
5 xul.dll mozilla::dom::OriginAttributesDictionary::operator= dom/bindings/ChromeUtilsBinding.cpp:1100
6 xul.dll mozilla::image::ImageCacheKey::ImageCacheKey image/ImageCacheKey.cpp:46
7 mozglue.dll wrap_strndup memory/build/mozmemory_wrap.cpp:80
8 xul.dll imgLoader::LoadImage image/imgLoader.cpp:2336
9 xul.dll nsContentUtils::LoadImage dom/base/nsContentUtils.cpp:3718

=============================================================

there's a handful of reports about these new crash signatures popping up in 62.0b4 so far. 
the crashing stacks all seem to go through a path that was touched by bug 1461921 in 62, in case that's related...

Comment 1

[Daniel Veditz [:dveditz]]

_WRITE and _EXEC violations don't look good, but I'm not sure how we'd get so far wrong we'd have an access violation in the  "{}" default constructor as in the crash linked to in comment 0.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

I wonder if this is a bug in the upgrade path in the registrar. The install age of all of the examples here is relatively short (< 20 minutes)...

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

I still can't find anything here. It is *extremely* odd that these are _WRITE violations. That makes me think that we're failing initializing one of the string members (my guess would be trying to refcount a StringBuffer), but I can't see any reason that would happen.

Comment 4

[Daniel Veditz [:dveditz]]

There have been no crashes in the last two weeks. In the last month only 6 people crashed 11 times, two on 60.0.2 and 4 on 62.0b4, all on July 1 or earlier.

On July 8 somebody crashed 4 times in a row using 62.0b5 on a probably related signature, mozilla::dom::ServiceWorkerRegistrationInfo::TransitionWaitingToActive (same symptom, bad write). The first crash was after 30 minutes of use but the following three were within a few seconds of startup. ALL of them have crazy stacks, with the crashing function called from different impossible places

bp-57ae8fb6-3b2d-499f-999f-0f40a0180708:  sandbox::PolicyRule::RebindCopy()
bp-661414cd-58b6-4a00-9755-b17e40180708:  msmpeg2vdec.dll@0x1c77ff
bp-4523c4f8-406c-443b-8e79-428dc0180708:  _TlgWrite (d3d11.dll)
bp-f29639ef-b89e-46df-8611-7dc0c0180708:  @0x1a586d3f

They all had reasonable looking mozilla::dom::ServiceWorkerRegistrationInfo::Activate() right before that. Is the symbolizer just confused about frame 2? In source the relevant line in Activate() calls TransitionWaitingToActive() directly.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Andrew, do you have any thoughts here?

Comment 6

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

(In reply to Daniel Veditz [:dveditz] from comment #4)
> On July 8 somebody crashed 4 times in a row using 62.0b5 on a probably
> related signature,
> mozilla::dom::ServiceWorkerRegistrationInfo::TransitionWaitingToActive (same
> symptom, bad write). The first crash was after 30 minutes of use but the
> following three were within a few seconds of startup. ALL of them have crazy
> stacks, with the crashing function called from different impossible places
> 
> bp-57ae8fb6-3b2d-499f-999f-0f40a0180708:  sandbox::PolicyRule::RebindCopy()
> bp-661414cd-58b6-4a00-9755-b17e40180708:  msmpeg2vdec.dll@0x1c77ff
> bp-4523c4f8-406c-443b-8e79-428dc0180708:  _TlgWrite (d3d11.dll)
> bp-f29639ef-b89e-46df-8611-7dc0c0180708:  @0x1a586d3f
> 
> They all had reasonable looking
> mozilla::dom::ServiceWorkerRegistrationInfo::Activate() right before that.
> Is the symbolizer just confused about frame 2? In source the relevant line
> in Activate() calls TransitionWaitingToActive() directly.

Yes, the symbolizer is confused.  If we click on the "Raw dump" tab for each crash we can see that the 2nd frame has "trust": "cfi_scan", except for "@0x1a586d3f" where it is "cfi" but it failed to find a corresponding symbol.  (Bug 929095 is still pending on exposing this information to the UI.)  Skimming the code at https://searchfox.org/mozilla-central/source/toolkit/crashreporter/google-breakpad/src/processor/stackwalker_x86.cc#441 and looking elsewhere briefly, it seems that it switches from "cfi" to "cfi_scan" when it can't find a module covering the instruction pointer currently inferred (for which it would have further CFI info), so it scans until it finds a "reasonable" looking return address on the stack.  Things do seem to get back on track in the frame or two after that, though, finding believable-on-the-first-try "cfi"-informed.

It's worth pointing out these crashes all report crash addresses that look 64-bit, but since they're 32-bit builds, I'm assuming there's just sign extension from 32-bit to 64-bit going on.  However, that still means the crashes all happen in writes to system address space, per docs like https://docs.microsoft.com/en-us/windows-hardware/drivers/gettingstarted/virtual-address-spaces#span-iduserspaceandsystemspacespanspan-iduserspaceandsystemspacespanspan-iduserspaceandsystemspacespanuser-space-and-system-space, which means something is badly wrong.


I've done some other investigations, but there's no obvious smoking gun from the small sample size.  I started to try and download the build so I could look at the disassembly around the alleged crash-point, but I now only seem able to easily download the windows .exe installer rather than being able to also get a zip file with the install byproducts.  (Maybe this changed with the switch from buildbot to taskcluster?)

But in summary, given the low rate of incidence I would suspect either a limited number of machines with issues, or this being a byproduct of a much more heinous memory corruption that spreads its crash stacks around a number of victim signatures.

Comment 7

[Liz Henry (:lizzard)]

Not seeing any recent crashes on beta and this issue seems stalled in any case. Marking fix-optional for beta 62.

Comment 8

[Marion Daly [:mdaly]]

:asuth, we should loop back around to this sometime in the next few weeks just to verify it's still stalled.

Comment 9

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

This is somewhat of an aside, but I ran :heycam's chkimg tool (https://github.com/heycam/chkimg) that's discussed in his blog post at https://mcc.id.au/blog/2018/07/memory-corruption and being productive for crash-stats in bug 1274628 against the raw dumps for a bunch of these crashes and the majority of them appear to have had some type of memory delta from what should be there.  Some of the deltas seemed consistent on a per-build basis though, so there still may be some bugs related to relocations or something similar.  (My checkout was from today.)

Which is mainly to say that after bug 1274628 and bug 929095 it will be easier to look into bugs like this because we can have more confidence that we're not chasing memory corruption.

Comment 10

[Jens Stutte [:jstutte]]

Since it is stalled, I lowered priority. Incidence seems very low now, so I guess we could even close it.

Comment 11

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

So, we do have one crash from 69.0.2 at https://crash-stats.mozilla.org/report/index/d2ee73f3-58b5-4445-b930-986d80191007 in shutdown that's happening in an iteration that should be safe because it's an nsTObserverArray. Given the low incidence rate, it's reasonable to suspect some type of memory corruption. I'd suggest leaving this at a P5 for now if only because I think it will be hard to dig this bug out and its history if the problem recurs and the function signatures in question still exist.

Comment 12

[Jens Stutte [:jstutte]]

If I query:
https://crash-stats.mozilla.org/search/?signature=%3Dmozilla%3A%3Adom%3A%3AServiceWorkerRegistrationInfo%3A%3AServiceWorkerRegistrationInfo&signature=%3Dmozilla%3A%3Adom%3A%3AServiceWorkerRegistrationInfo%3A%3AUpdateRegistrationState&product=Firefox&date=%3E%3D2019-01-01T00%3A00%3A00.000Z&date=%3C2019-11-29T14%3A58%3A00.000Z&_facets=install_time&_facets=version&_facets=address&_facets=moz_crash_reason&_facets=reason&_facets=build_id&_facets=platform_pretty_version&_facets=signature&_facets=useragent_locale&_sort=-date&_columns=date&_columns=signature&_columns=reason&_columns=product&_columns=version&_columns=build_id&_columns=platform#crash-reports

I see different exception types in similar places. I am not too "optimistic", that all of these can be explained by physical memory corruptions. They might be also sign of a corrupted stack. In particular there is an EXCEPTION_STACK_BUFFER_OVERRUN. And if I look into this: https://crash-stats.mozilla.org/report/index/0550f2d9-d91c-472c-8715-60f720190722 I see a corrupted stack. If I assume that the stack was sane when we entered that function: https://searchfox.org/mozilla-central/source/dom/serviceworkers/ServiceWorkerRegistrationInfo.cpp#401, I find a list on the stack (https://searchfox.org/mozilla-central/source/dom/serviceworkers/ServiceWorkerRegistrationInfo.cpp#407) as the only potentially "suspicious" variable on the stack of this function that might cause problems. But the code looks harmless (to me), so I have no idea, actually.

:Daniel, I see some recent crashes. Should we take this more serious?

Comment 13

[Daniel Veditz [:dveditz]]

I think we should close this bug INCOMPLETE and unhide it. I don't think we're making any progress here and the volume is super low.

Comment 14

[Jens Stutte [:jstutte]]

As suggested by :dveditz, I close this bug. Not sure about eventual steps to be undertaken before removing sec-high.

Comment 15

[Andrew McCreight [:mccr8]]

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

As suggested by :dveditz, I close this bug. Not sure about eventual steps to be undertaken before removing sec-high.

We usually just leave things like this as sec-high but closed. There's still some issue out there that is sec-high. We just can't act on it.

Comment 16

[David Lawrence [:dkl]]

Removing employee no longer with company from CC list of private bugs.