Closed Bug 857700 Opened 7 years ago Closed 7 years ago

TEST-UNEXPECTED-FAIL | /builds/slave/m-cen-osx64-d-0000000000000000/build/js/src/jit-test/tests/asm.js/testModuleFunctions.js | --no-jm: AsmJSMachExceptionHandlerThread: mach_msg failed with 268451842

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23
Tracking Status
firefox22 --- fixed
firefox23 --- fixed

People

(Reporter: neil, Assigned: luke)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #851421 +++

https://tbpl.mozilla.org/php/getParsedLog.php?id=21391058&tree=Firefox
Assignee: general → luke
Attached patch fixSplinter Review
Thinking about this code a bit more, there is an obvious race condition that I can reproduce locally with a sleep() and I'm pretty sure is happening here with low frequency: the main runtime starts and finishes before the Mach exception handler thread starts.  Thus, the mach_msg listens on an invalid port so we return an error (triggering the intentional MOZ_CRASH).

The fix is to join on the thread before deallocating the port.  It turns out the pthread_cancel was totally bogus and not really canceling the thread (mach_msg doesn't care, it'll keep on blocking, even with PTHREAD_CANCEL_ASYNCHRONOUS).  After considering a bunch of different things, the simplest solution I found was to send a "quit" message to the mach exception handler thread.

While doing this, I discovered some terrible Mach interface hazards that are fixed here also:
 - mach_thread_self() increments a ref-count and needs mach_port_deallocate
 - mach_port_deallocate only decrements the "send right" ref-count, not the "receive right" ref-count, which means an allocated port (and the backing message queue) won't be deallocated!  One must either mach_port_mod_refs or mach_port_destroy.  (Everyone seems to get this wrong.)

https://tbpl.mozilla.org/?tree=Try&rev=7e1b0b568f0d
Comment on attachment 739411 [details] [diff] [review]
fix

I think Vlad knows this code the best.  (Vlad see previous comment for the explanation.)
Attachment #739411 - Flags: review?(vladimir)
https://hg.mozilla.org/mozilla-central/rev/d3a6b7822cdb
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment on attachment 739411 [details] [diff] [review]
fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 840282
User impact if declined: crash on OSX
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low, asm.js-only
Attachment #739411 - Flags: approval-mozilla-aurora?
Attachment #739411 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.