Closed Bug 1633469 Opened 7 months ago Closed 1 month ago

Crash in [@ mozilla::dom::`anonymous namespace'::GetSubscriptionCallback::OnPushSubscription]

Categories

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

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- wontfix
firefox82 --- wontfix
firefox83 --- wontfix
firefox84 --- fixed

People

(Reporter: gsvelto, Assigned: sg, NeedInfo)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-2649db29-1fcb-4ed5-b436-3040f0200427.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::`anonymous namespace'::GetSubscriptionCallback::OnPushSubscription dom/push/PushManager.cpp:177
1 xul.dll mozilla::dom::`anonymous namespace'::GetSubscriptionRunnable::Run dom/push/PushManager.cpp
2 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1200
3 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:481
4 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87
5 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
6 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290
7 xul.dll nsBaseAppShell::Run widget/nsBaseAppShell.cpp:137
8 xul.dll nsAppShell::Run widget/windows/nsAppShell.cpp:406
9 xul.dll XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:909

Thist started with buildid 20200409095500, looks like a regression from bug 1620152.

This is probably due to the dispatch happening when the worker is already shutting down.

Not sure if https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/dom/push/PushManager.cpp#177 should rather simply be changed to

if (!r->Dispatch()) {
  return NS_ERROR_UNEXPECTED;
}

This seems to be called from https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/dom/push/PushManager.cpp#184, which is already an error handler that ignores the result.

Flags: needinfo?(bugmail)

This might be related to bug 1435343: The Dispatch() function returns false and GetSubscriptionResultRunnable derives from WorkerRunnable, which implements a suspicious PreDispatch().

Edit: Suspicious probably only in the shutdown case, where we might see a race between the main thread and the worker thread? I remember to have seen some assertions that indicate something like this on a different bug, need to check back.

See Also: → 1435343
Flags: needinfo?(jstutte)

Found, and maybe related: bug 1611094 ?

firefox77 = affected
firefox76 (beta) = fix-optional because it's not crashing (because 76 doesn't have the diagnostic assertion), but it might be affected the underlying worker shutdown problem.

Maybe another case of event loop spinning during shutdown?

Flags: needinfo?(jstutte)
See Also: → 1633342
Duplicate of this bug: 1637359
Duplicate of this bug: 1452215

Thank you! I am adding the signature to make the bug appear in the crash triage list.

Crash Signature: [@ mozilla::dom::`anonymous namespace'::GetSubscriptionCallback::OnPushSubscription] → [@ mozilla::dom::`anonymous namespace'::GetSubscriptionCallback::OnPushSubscription] [@ mozilla::dom::(anonymous namespace)::GetSubscriptionCallback::OnPushSubscription]
Severity: -- → S3
Priority: -- → P3

This appears to have spiked over the past few weeks. Jens, can you take a look?

Flags: needinfo?(jstutte)

Looking at the one and only use of OnPushSubscription, it seems that possible error returns are expected at least in theory. I think it is worth a try to follow Simon's suggestion from comment 1. Simon, can you give this a spin?

Flags: needinfo?(jstutte) → needinfo?(sgiesecke)
Assignee: nobody → sgiesecke
Status: NEW → ASSIGNED

I provided a patch along the lines of comment 1.

Flags: needinfo?(sgiesecke)
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f857195f8c1
Propagate failure of Dispatch in GetSubscriptionCallback::OnPushSubscription. r=jstutte

While the patch removes the crash here, this will just transform into another error situation. I noticed that quite some of the URLs (where they have been reported) are from youtube.com here.

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
You need to log in before you can comment on or make changes to this bug.