Closed Bug 1298661 Opened 3 years ago Closed 3 years ago

Crashes in NS_CycleCollectorSuspect3 when releasing mozilla::dom::IDBTransaction in workers

Categories

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

50 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: smaug, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Attachments

(2 files)

Testcase is in https://bugzilla.mozilla.org/show_bug.cgi?id=1151643#c8

I'm pretty sure the issue is that we have syncloop in workers because of XHR, and during that we dispatch some dom event in worker which causes IDB to use MetaStableState, at level 3, but workers have http://searchfox.org/mozilla-central/rev/f80822840bc5f3d2d3cae3ece621ddbce72e7f54/dom/workers/RuntimeService.cpp#963

I think, the main issue is that we dispatch DOM events while we have still syncloop on stack.
But perhaps we could just remove the limitation in RuntimeService.cpp (but that feels a bit hack).
Severity: normal → critical
Keywords: crash
This is a bit hackish, but should bring back some of the behavior bug 1179909 broke. We really need to handle metastable stuff while having the sync loop on stack. (or we need to change how XHR works in workers, but that is larger change).

Some of this stuff will change when Bug 1193394 brings us real microtasks even for Promises (right now we have odd almost-like-task-microtasks).
But I think we need to fix the crash even on branches, so the patch should be about the safest possible option I could think of.



https://treeherder.mozilla.org/#/jobs?repo=try&revision=ceb08ca40d90
Assignee: nobody → bugs
Attachment #8786446 - Flags: review?(amarchesini)
Comment on attachment 8786446 [details] [diff] [review]
meta_stable_crash_in_workers.diff

Review of attachment 8786446 [details] [diff] [review]:
-----------------------------------------------------------------

"This is a bit hackish,...", I totally agree but I cannot suggest anything better.

::: xpcom/base/CycleCollectedJSRuntime.h
@@ +335,5 @@
>      MOZ_ASSERT(mJSContext);
>      return JS::RootingContext::get(mJSContext);
>    }
>  
> +  bool MicroTaskCheckpointDisabled()

const
Attachment #8786446 - Flags: review?(amarchesini) → review+
Attached patch + constSplinter Review
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/45d53e85bd0d
let meta-stable state runnables run during sync loops, r=baku
https://hg.mozilla.org/mozilla-central/rev/45d53e85bd0d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.