1375412 - Various crashes in Mozilla Firefox Nightly 56.0a1 (2017-06-21) after landing patches from bug #1323069

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

[Tracking Requested - why for this release]: Regression


"Speedy" Regression window (mozilla-central)
Good:
https://ftp.mozilla.org/pub/firefox/nightly/2017/06/2017-06-20-03-02-08-mozilla-central/

Bad:
https://ftp.mozilla.org/pub/firefox/nightly/2017/06/2017-06-21-10-23-01-mozilla-central/

Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7a6baa6cca3292e8099e652b64d27e74df560874&tochange=92eb911c35da48907d326604c4c92cf55e551895

Probably caused by:
bug #1372453 [Core:XPCOM]-Better naming for ProxyReleaseEvent
bug #1374594 [Core:XPCOM]-Construct MonitorAutoUnlock from a MonitorAutoLock
bug #833143  [Core:XPCOM]-Don't GC in nsXREDirProvider::DoShutdown()

Any ideas which one or it's missed call?

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Can't be "my" bug 1374594, as it was only adding some constructors that are not even used yet.

(At a glance, bug 833143 also seems fairly innocuous, so I'd bet on bug 1372453 as a more likely candidate)

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

nsProxyReleaseEvent is a runnable inheriting from mozilla::Runnable directly instead of creating from RunnableMethodImpl class.
I didn't see any suspicious point yet that bug 1372453 could cause this regression.

Comment 3

[Nathan Froyd [:froydnj]]

None of those bugs seem responsible.  All of the crashes seem to be in runnables coming from dom/media/systemservices/CameraChild.cpp (hard to tell exactly where, because the debug information is unhelpful), but I'd be looking for changes under dom/media/ as being responsible (a WebRTC change, too, maybe?).

Comment 4

[Nathan Froyd [:froydnj]]

Clearing ni? for mccr8, too.

Comment 5

[Andrew McCreight [:mccr8]]

Maybe bug 1372453 or bug 1372405 caused a signature change? There's nothing obviously related to media recording that I can see in that range in comment 1.

Comment 6

[Andrew McCreight [:mccr8]]

Err, in comment 0.

Comment 7

[Emanuel Hoogeveen [:ehoogeveen]]

This is a near consistent startup crash with my main profile. My own bisection points to bug 1323069 as the culprit:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8f2434d1648b623c06326f582fd431cbe533c6a8&tochange=7bdf1aa4e51482e51c0104845dc249cdc027a809

Aaron, can you make anything of this signature? Is bug 1323069 likely to be responsible?

Comment 8

[Emanuel Hoogeveen [:ehoogeveen]]

Found the corresponding Tinderbox builds for a 2nd confirmation.

Last good:
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win64/1497983903/

First bad:
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win64/1497985823/

If it helps, I have accessibility.force_disabled set to true (since it kept disabling e10s for me on Windows 10 - probably not needed now).

Comment 9

[Marcia Knous [:marcia]]

Tracking 56+ - crash regression that is #25 top browser crash on nightly.

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Use a custom runnable for dispatching LazyInstantiator::AccumulateTelemetry

We weren't holding a strong reference before. In my mental model, this would be okay because the runnable that dispatches AccumulateTelemetry would be enclosed by the strong ref held by the one that dispatched GatherTelemetry.

That clearly isn't holding.

Instead I have made a custom runnable that holds a strong ref to the lazy instantiator and only ever releases it on the main thread.

Comment 13

[Eitan Isaacson [:eeejay]]

Comment on attachment 8880914 [details] [diff] [review]
Use a custom runnable for dispatching LazyInstantiator::AccumulateTelemetry

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

Looks good.

Comment 14

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/921156a76d80aee61af486e0fc75a8e12d788898
Bug 1375412: Strengthen ownership of a11y::LazyInstantiator when posting runnable back to main thread; r=eeejay

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/921156a76d80

Comment 16

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm marking this bug as VERIFIED, because starting from build 2017-06-27, there are no crashes anymore.

Thank you Aaron Klotz [:aklotz] very much for fixing this very fast! \o/

Comment 17

[Emanuel Hoogeveen [:ehoogeveen]]

Yes, I can confirm that these crashes are fixed for me as well. I couldn't use Nightly at all before (got a startup crash every time), haven't crashed since this landed.