1425509 - Crash in performance.timeOrigin

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Accessing performance.timeOrigin in a worker crashes in some cases. It's hard to reproduce - I was unable to do it on Linux, but running locally on a Mac I can get the following test (from Bug 1424341) to reliably crash.

I can't get a nice callstack anymore, but here it is manually:

GECKO(28216) | Assertion failure: NS_IsMainThread(), at /Users/tritter/Documents/moz/mozilla-unified/obj-x86_64-apple-darwin17.2.0/dist/include/mozilla/ClearOnShutdown.h:102

https://searchfox.org/mozilla-central/rev/110706c3c09d457dc70293b213d7bccb4f6f5643/xpcom/base/ClearOnShutdown.h#102
https://searchfox.org/mozilla-central/rev/110706c3c09d457dc70293b213d7bccb4f6f5643/dom/performance/PerformanceService.cpp#26
https://searchfox.org/mozilla-central/rev/110706c3c09d457dc70293b213d7bccb4f6f5643/dom/performance/Performance.cpp#135

It seems like a relatively straightforward error, but I'm not sure what the correct resolution is.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(Nothing to do with DOM Events.)

Comment 2

[Andrea Marchesini [:baku]]

Attachment: preferenceService initialized in RuntimeService

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8937464 [details] [diff] [review]
preferenceService initialized in RuntimeService

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

Looks ok.  It might be slightly nicer to make a PerformanceService::Init() called from our process startup code and then just a PerformanceService::Get() instead of GetOrCreate().  Or even a PerformanceService::Init() and a static method for TimeOrigin().

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/59658470f635
Initialize PerformanceService in RuntimeService, r=bkelly

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

FWIW, I can confirm this fixed my problem :)

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/59658470f635

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

[Tracking Requested - why for this release]: This is required for Bug 1424341 to land in 58

Existing patch applies cleanly to -beta for me.

Comment 8

[Gerry Chang [:gchang]]

Track 58+ as this is required by bug 1424341.

Comment 9

[Gerry Chang [:gchang]]

Hi Tom, since this is required by bug 1424341, please nominate beta uplift request.

Comment 10

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8937464 [details] [diff] [review]
preferenceService initialized in RuntimeService

Approval Request Comment
[Feature/Bug causing the regression]: We would like the ability to easily tune timer precision on beta for the next release cycle to respond to Spectre. The pref is turned off by default right now.

[User impact if declined]: We will have to do dot releases instead of doing a system addon to tweak a pref. Also, the dot releases will be more work (much more work if we don't uplift this in one of them) to cover all timers - previously our chemspill was only to address performance.now()

[Is this code covered by automated tests?]: Yes (in fact it fixes a test and stops it from crashing.)

[Has the fix been verified in Nightly?]: No

[Needs manual test from QE? If yes, steps to reproduce]: No

[List of other uplifts needed for the feature/fix]: This is a dependency for 1424341

[Is the change risky?]: No

[Why is the change risky/not risky?]: This is a small patch that initializes an object (if i is not already initialized) on WorkerThreads.

[String changes made/needed]: None

Comment 11

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Sorry this has been verified in Nightly actually.

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8937464 [details] [diff] [review]
preferenceService initialized in RuntimeService

Required by bug 1424341. Beta58+.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/102edbb34a09b8dcfb100ce96d765daa5ace091a (FIREFOX_58b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/f2c70b728158