1317259 - [App Verifier] Critical section already initialized in mozilla::TimeStamp::Startup

Status: RESOLVED FIXED

(Core :: mozglue, defect, P1)

Description

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: App verifier error report

App verifier reports that a CRITICAL_SECTION is initialize twice in running
./mach mochitest-browser

Stack trace:
vfbasics!+7ffab83f6d61 ( @ 0)
KERNELBASE!InitializeCriticalSectionAndSpinCount+a ( @ 0)
mozglue!mozilla::TimeStamp::Startup+5f (e:\hg\mozilla-central\mozglue\misc\timestamp_windows.cpp @ 493)
xul!NS_InitMinimalXPCOM+17 (e:\hg\mozilla-central\xpcom\build\xpcominit.cpp @ 788)
xul!mozilla::gfx::GPUParent::Init+bd (e:\hg\mozilla-central\gfx\ipc\gpuparent.cpp @ 101)
xul!XRE_InitChildProcess+5b9 (e:\hg\mozilla-central\toolkit\xre\nsembedfunctions.cpp @ 669)
firefox!content_process_main+8d (e:\hg\mozilla-central\ipc\contentproc\plugin-container.cpp @ 198)
firefox!NS_internal_main+b1 (e:\hg\mozilla-central\browser\app\nsbrowserapp.cpp @ 392)
firefox!wmain+163 (e:\hg\mozilla-central\toolkit\xre\nswindowswmain.cpp @ 118)
firefox!__scrt_common_main_seh+11d (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 253)
KERNEL32!BaseThreadInitThunk+14 ( @ 0)
ntdll!RtlUserThreadStart+21 ( @ 0)

I think before this, TimeStamp::Startup() is called in TimeStamp::ProcessCreation().

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Bug 1317259 - Prevent double init in mozilla::TimeStamp::Startup() on Windows.

Review commit: https://reviewboard.mozilla.org/r/92714/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92714/

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8810337 [details]
Bug 1317259 - Prevent double init in mozilla::TimeStamp::Startup() on Windows.

https://reviewboard.mozilla.org/r/92714/#review92812

Thank you!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/392c92c5a374
Prevent double init in mozilla::TimeStamp::Startup() on Windows. r=froydnj

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/392c92c5a374

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Let it ride the train.

Comment 6

[Cervantes Yu [:cyu] [:cervantes]]

Comment on attachment 8810337 [details]
Bug 1317259 - Prevent double init in mozilla::TimeStamp::Startup() on Windows.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1294350
[User impact if declined]: Possibly stability issues.
[Is this code covered by automated tests?]: No. The impact of this bug is undefined behavior. Very likely the bug is not observable, or the symptom is hard to correlate with this bug. To add automatic test, we need to add some test-only code for the test case, which may be an overkill.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No. This needs to be verified with debugger or dynamic check tool.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This adds a simple check to the Windows implementation of TimeStamp. The check is similar to the implementation on other platforms. It's obvious that this patch is not risky.
[String changes made/needed]: No.

Comment 7

[Julien Cristau [:jcristau]]

Comment on attachment 8810337 [details]
Bug 1317259 - Prevent double init in mozilla::TimeStamp::Startup() on Windows.

This landed on trunk when that was 53, so clearing aurora approval flag.  Let's fix this in beta52.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/47beb6fad37a