Closed Bug 1509585 Opened 1 year ago Closed 1 year ago
Crash in mozilla::dom::Worker
47 bytes, text/x-phabricator-request
|Details | Review|
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?
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
This looks like a null ptr deref, not a use-after-free.
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.
Please request Beta approval on this when you get a chance.
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:
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+
You need to log in before you can comment on or make changes to this bug.