1675973 - Assertion failure: false (r->Dispatch()), at src/dom/push/PushManager.cpp:339

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect, P3)

Description

[Tyson Smith [:tsmith]]

No test case is available. A Pernosco session is available here: https://pernos.co/debug/p0jeIc1nHniFLBDwuVmvZQ/index.html

Assertion failure: false (r->Dispatch()), at src/dom/push/PushManager.cpp:339

#0 0x7f334899772b in mozilla::dom::(anonymous namespace)::PermissionStateRunnable::Run() /gecko/dom/push/PushManager.cpp:339:5
#1 0x7f33419a6f79 in mozilla::RunnableTask::Run() /gecko/xpcom/threads/TaskController.cpp:450:16
#2 0x7f33419a3a37 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:720:26
#3 0x7f33419a1977 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:579:15
#4 0x7f33419a1dcd in mozilla::TaskController::ProcessPendingMTTask(bool) /gecko/xpcom/threads/TaskController.cpp:373:36
#5 0x7f33419aea61 in operator() /gecko/xpcom/threads/TaskController.cpp:120:37
#6 0x7f33419aea61 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_3>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
#7 0x7f33419cf49b in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1197:14
#8 0x7f33419da19c in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:513:10
#9 0x7f3342cae8ef in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:87:21
#10 0x7f3342bb05a1 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
#11 0x7f3342bb05a1 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
#12 0x7f3342bb05a1 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
#13 0x7f33499bd427 in nsBaseAppShell::Run() /gecko/widget/nsBaseAppShell.cpp:137:27
#14 0x7f334d6fb1ff in XRE_RunAppShell() /gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
#15 0x7f3342bb05a1 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
#16 0x7f3342bb05a1 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
#17 0x7f3342bb05a1 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
#18 0x7f334d6fa79c in XRE_InitChildProcess(int, char**, XREChildData const*) /gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
#19 0x55cfbf2de55d in content_process_main(mozilla::Bootstrap*, int, char**) /gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#20 0x55cfbf2de997 in main /gecko/browser/app/nsBrowserApp.cpp:304:18

Comment 1

[Jens Stutte [:jstutte]]

This Dispatch can legitimately go wrong if the worker is already shutting down. So we should not assert here.

I assume we should just propagate the error, but maybe we should have a specific error code for this kind of situations? And check other instances of WorkerRunnable::Dispatch calls?

Comment 2

[Jens Stutte [:jstutte]]

Looking at this a bit closer, it seems that we:

• dispatch a PermissionStateRunnable from the worker thread to the main thread (and the worker is still alive at this time)
• find our worker shutting down when we execute the PermissionStateRunnable on the main thread and want to return our state to the worker thread.

So it seems that we should definitely propagate an error in this case and ensure a proper cleanup (rejection of the promise) in PushManager::PermissionState like for the failure of the proxy creation.

WDYT, Eden?

Comment 3

[Jens Stutte [:jstutte]]

Yaron, restoring your ni? given that Eden will not be able to look at this soon.

Comment 4

[Yaron Tausky [:ytausky]]

The promise can only be rejected on the worker thread, so if we can't dispatch to it then there's nothing we can do about it. I think in this case just removing the assertion would be fine.

Comment 5

[Yaron Tausky [:ytausky]]

Attachment: Bug 1675973 - Remove wrong assertion in PushManager.cpp r=#dom-workers-and-storage-reviewers

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/72b1f78db9b7
Remove wrong assertion in PushManager.cpp r=dom-workers-and-storage-reviewers,edenchuang

Comment 7

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/72b1f78db9b7