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/test Module Functions .js | --no-jm: Asm JSMach Exception Handler Thread: mach _msg failed with 268451842
+++ This bug was initially created as a clone of Bug #851421 +++ https://tbpl.mozilla.org/php/getParsedLog.php?id=21391058&tree=Firefox
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)
Attachment #739411 - Flags: review?(vladimir) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
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.