Closed Bug 691046 Opened 8 years ago Closed 8 years ago

worker.postMessage behaves like there is a hidden exception in Cloud9 Ace

Categories

(Core :: DOM: Core & HTML, defect)

8 Branch
x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla8
Tracking Status
firefox8 + fixed
firefox9 + fixed
firefox10 --- fixed

People

(Reporter: fabian.jakobs, Assigned: bent.mozilla)

Details

(Keywords: regression, Whiteboard: [qa!])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_1) AppleWebKit/535.5 (KHTML, like Gecko) Chrome/16.0.891.1 Safari/535.5

Steps to reproduce:

The latest FF 8 beta broke the Ace editor <http://ace.ajax.org>. I was able to trace it down to a problem with web workers and postMessage. The relevant code is:

console.log("before")
this.$worker.postMessage({event: event, data: data});
console.log("posted");

The last line is never executed, however there is no error shown and the callstack is rolled back just normal. It looks like an invisible exception is thrown.

Steps to reproduce:

1. go to <http://fjakobs.github.com/ace_ff8_bug/kitchen-sink.html>
2. execute fire() in the console.
3. Observe that "before" is printed but "posted" is not.

Setting a breakpoint in <http://fjakobs.github.com/ace_ff8_bug/demo/kitchen-sink-uncompressed.js> line 17245 and stepping over the postMessage shows this behavior.
Component: General → DOM
Product: Firefox → Core
QA Contact: general → general
the related Ace bug is <https://github.com/ajaxorg/ace/issues/405>
Yeah, there's a bug here, in our code as well as yours I think. You're calling postMessage() on a worker after you call terminate() on it which is probably not what you mean to do. However, the bigger problem here is that we throw an uncatchable exception in this case.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch Patch, v1Splinter Review
Patch should fix us.
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Attachment #564078 - Flags: review?(jonas)
Calling postMessage() after terminate() is really not what I intended. Let me know once you have a build where I can test the fix. The bug is currently in FF 8beta and I think it is pretty severe.
Comment on attachment 564078 [details] [diff] [review]
Patch, v1

This fixes a pretty bad bug in the new worker implementation (new in FF8) and it's a tiny change, well localized.
Attachment #564078 - Flags: approval-mozilla-beta?
Attachment #564078 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/74e93efada21
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
Attachment #564078 - Flags: approval-mozilla-beta?
Attachment #564078 - Flags: approval-mozilla-beta+
Attachment #564078 - Flags: approval-mozilla-aurora?
Attachment #564078 - Flags: approval-mozilla-aurora+
qa+ for verification in Firefox 8, 9, and 10 using the testcase in comment 0.
Whiteboard: [qa+]
I tested to see if the issue is still reproducible since the fix landed on all channels (comment 7 and comment8); I followed the steps to reproduce in the description and got the following results:
WORKS:
-> Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0 (Mac OS 10.6 - Beta)
-> Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a2) Gecko/20111013 Firefox/9.0a2 (Mac OS 10.6 - Aurora)
-> Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a1) Gecko/20111013 Firefox/10.0a1 (Mac OS 10.6 - Nightly)

Since it works on all channels setting this bug as Verified and changing the whiteboard entry qa+ to qa!
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] → [qa!]
[triage comment]

We'll track this as it was a regression in a widely used library and we'll want to be aware if it breaks or wasn't fixed all the way. Because it is already fixed in beta, nothing to do currently.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.