Assertion failure: mActorDestroyed, at dom/asmjscache/AsmJSCache.cpp

RESOLVED FIXED in Firefox 46

Status

()

Core
DOM
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jesse Ruderman, Assigned: baku)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla46
assertion, testcase
Points:
---

Firefox Tracking Flags

(firefox45 affected, firefox46 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
Created attachment 8682910 [details]
testcase

Assertion failure: mActorDestroyed, at dom/asmjscache/AsmJSCache.cpp:1277
(Reporter)

Comment 1

2 years ago
Created attachment 8682911 [details]
stack

Comment 2

2 years ago
Guessing this is a regression from bug 1173756?

Comment 3

2 years ago
(In reply to Luke Wagner [:luke] from comment #2)
> Guessing this is a regression from bug 1173756?

Yeah, if BlockUntilOpen() fails early then the IPC protocol is not created and mActorDestroyed is not cleared. I'll fix it.
(Assignee)

Comment 4

2 years ago
Created attachment 8705601 [details] [diff] [review]
bug_asmjs_1221418.patch
Assignee: nobody → amarchesini
Attachment #8705601 - Flags: review?(jvarga)

Comment 5

2 years ago
Comment on attachment 8705601 [details] [diff] [review]
bug_asmjs_1221418.patch

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

I don't understand this fix, mActorDestroyed(true) by default ?

I would rather do:
1. Create NoteActorDestroyed() method that sets mActorDestroyed to true
2. Create Cleanup() method that calls NoteActorDestroyed(), the call can be #ifdef DEBUG'ed since it's only useful to silent the assertion in the destructor
3. Update ActorDestroyed() to call NoteActorDestroyed() instead of setting the flag directly
4. Add childRunnabled->Cleanup() if BlockUntilOpen() fails
Attachment #8705601 - Flags: review?(jvarga)
(Assignee)

Comment 6

2 years ago
> I don't understand this fix, mActorDestroyed(true) by default ?

what's the point to have mActorDestroyed(false) by default if we don't have an actor yet?
(Assignee)

Comment 7

2 years ago
Created attachment 8705615 [details] [diff] [review]
bug_asmjs_1221418.patch
Attachment #8705601 - Attachment is obsolete: true
Attachment #8705615 - Flags: review?(jvarga)
(Assignee)

Comment 8

2 years ago
Created attachment 8705619 [details] [diff] [review]
bug_asmjs_1221418.patch
Attachment #8705615 - Attachment is obsolete: true
Attachment #8705615 - Flags: review?(jvarga)
Attachment #8705619 - Flags: review?(jvarga)

Comment 9

2 years ago
Comment on attachment 8705619 [details] [diff] [review]
bug_asmjs_1221418.patch

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

Muchas gracias!
Attachment #8705619 - Flags: review?(jvarga) → review+

Comment 10

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/273505970b6c

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/273505970b6c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.