Closed Bug 1539535 Opened 3 years ago Closed 3 years ago

Crash in [@ mozilla::dom::RemoteWorkerChild::CloseWorkerOnMainThread]

Categories

(Core :: DOM: Workers, defect, P1)

67 Branch
All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 + fixed
firefox68 --- fixed

People

(Reporter: philipp, Assigned: perry)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-04f73f3a-30be-4ba8-8312-3b3400190327.

Top 10 frames of crashing thread:

0 xul.dll void mozilla::dom::RemoteWorkerChild::CloseWorkerOnMainThread dom/workers/remoteworkers/RemoteWorkerChild.cpp:459
1 xul.dll nsresult mozilla::detail::RunnableFunction<`lambda at z:/task_1553215353/build/src/dom/workers/remoteworkers/RemoteWorkerChild.cpp:525:9'>::Run xpcom/threads/nsThreadUtils.h:562
2 xul.dll nsresult mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:295
3 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1179
4 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:482
5 xul.dll void mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:88
6 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
7 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
8 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
9 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:411

crash reports with this signature and MOZ_RELEASE_ASSERT(lock->mWorkerPrivate) added in bug 1530223 are newly appearing in firefox 67.

Priority: -- → P3

These crashes are new in 67 and are caused by the fix for the crasher bug 1530223 but the volume looks equivalent. Andrew can we get this investigated please? Thanks.

No longer blocks: 1530223
Regressed by: 1530223
Flags: needinfo?(overholt)

This is caused by a race between RemoteWorkerChild::CloseWorkerOnMainThread and RemoteWorkerChild::ShutdownOnWorker. This should be fixed by one of the patches from bug 1231213, but until all that stuff has landed it may be worth just returning from the function if (!lock->mWorkerPrivate) instead of MOZ_RELEASE_ASSERTing.

During our team meeting tomorrow we will discuss who can take Perry's suggestion from comment 2.

Flags: needinfo?(overholt)
Assignee: nobody → perry

Tracking for 67 as the crashes spiked in beta 8 (138 crashes which is more crashes than betas 1 to 7 combined)

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See How Do You Triage for more information

Priority: P3 → P1

Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/18af3ac7686b
Add a (temporary) null check in RemoteWorkerChild::CloseWorkerOnMainThread r=asuth

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Perry, could you request an uplift to beta so as that we get it in the next beta? I don't think that there is enough volume on Nightly to evaluate if the fix works while we have this volume of crashes on beta. Thanks

Flags: needinfo?(perry)

Comment on attachment 9057059 [details]
Bug 1539535 - Add a (temporary) null check in RemoteWorkerChild::CloseWorkerOnMainThread r?asuth

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1530223
  • User impact if declined: Browser may crash non-deterministically.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch changes an assertion that asserts a non-null pointer to a check that handles a null pointer accordingly.
  • String changes made/needed:
Flags: needinfo?(perry)
Attachment #9057059 - Flags: approval-mozilla-beta?

Comment on attachment 9057059 [details]
Bug 1539535 - Add a (temporary) null check in RemoteWorkerChild::CloseWorkerOnMainThread r?asuth

Fix for a crash spiking in 67 beta but with no volume on 68, uplift approved for 67 beta 11 as we need the beta volume to understand if the patch has an impact on crashes.

Attachment #9057059 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.