Closed Bug 1317259 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: mozglue, defect, P1)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: cyu, Assigned: cyu)

References

(Blocks 1 open bug)

Details

(Whiteboard: app_verifier)

Attachments

(2 files)

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().
Summary: [App Verifier] Critical section already initialized → [App Verifier] Critical section already initialized in mozilla::TimeStamp::Startup
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!
Attachment #8810337 - Flags: review?(nfroyd) → review+
Assignee: nobody → cyu
Pushed by cyu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/392c92c5a374
Prevent double init in mozilla::TimeStamp::Startup() on Windows. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/392c92c5a374
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Let it ride the train.
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.
Attachment #8810337 - Flags: approval-mozilla-beta?
Attachment #8810337 - Flags: approval-mozilla-aurora?
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.
Attachment #8810337 - Flags: approval-mozilla-beta?
Attachment #8810337 - Flags: approval-mozilla-beta+
Attachment #8810337 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.