Closed Bug 1509585 Opened 1 year ago Closed 1 year ago

Crash in mozilla::dom::WorkerPrivate::Notify

Categories

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

61 Branch
Unspecified
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- unaffected
firefox65 + fixed
firefox66 + fixed

People

(Reporter: philipp, Assigned: mrbkap)

References

Details

(Keywords: crash, csectype-nullptr, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-2e06cd13-3fa6-471f-9db7-e1d460181123.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::WorkerPrivate::Notify dom/workers/WorkerPrivate.cpp:1699
1 xul.dll mozilla::dom::RemoteWorkerChild::CloseWorkerOnMainThread dom/workers/remoteworkers/RemoteWorkerChild.cpp:515
2 xul.dll mozilla::dom::workerinternals::RuntimeService::UnregisterWorker dom/workers/RuntimeService.cpp:1536
3 xul.dll ?Run@TopLevelWorkerFinishedRunnable@?A0x30C6BC27@dom@mozilla@@EEAA?AW4nsresult@@XZ$3a0bfecfb3064f89e01e6f0b017f7aa0 dom/workers/WorkerPrivate.cpp:295
4 xul.dll mozilla::ThrottledEventQueue::Inner::Executor::Run xpcom/threads/ThrottledEventQueue.cpp:84
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1244
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:530
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:97
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:318
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:298

=============================================================

this content crash signature is regressing on 65.0a1 since build 20181120100045, so timing wise that looks related to 1438945. so far the crash is only reported from installations on windows 7.
This looks like another use-after-free, at least from a quick look. :mrbkap, could you please have a look to see if something pops up?
Group: core-security
Flags: needinfo?(mrbkap)
Priority: -- → P1
Group: core-security → dom-core-security
All of the crashes are at 0x10 (or 0xc), which suggests a null pointer somewhere. I'll dig in more here.
Assignee: nobody → mrbkap
Flags: needinfo?(mrbkap)
This looks like a null ptr deref, not a use-after-free.
Group: dom-core-security
This code races with the WeakWorkerRef shutdown code that sets both `mWorkerState` and `mWorkerPrivate` (though on different threads). This patch is based on the observation that except for failure cases, we can't get to `RuntimeServiceWorker::UnregisterWorker` without having already notified the `WorkerRef`s.
https://hg.mozilla.org/mozilla-central/rev/cbfa24a98e21
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Please request Beta approval on this when you get a chance.
Flags: needinfo?(mrbkap)
Comment on attachment 9030530 [details]
Bug 1509585 - Remove some redundant worker shutdown code. r=asuth

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1438945

User impact if declined: Random crashes on websites using shared workers.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

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): We have audited the code and the patch simply removes a call that isn't needed any more.

String changes made/needed:
Flags: needinfo?(mrbkap)
Attachment #9030530 - Flags: approval-mozilla-beta?
Comment on attachment 9030530 [details]
Bug 1509585 - Remove some redundant worker shutdown code. r=asuth

[Triage Comment]
Removes some redundant code to fix random crashes on websites using shared workers. Approved for 65.0b7.
Attachment #9030530 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1530223
You need to log in before you can comment on or make changes to this bug.