Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d502f1d70df1

Splitting this patch into two parts: the operator== consting and the exclusiveAccessOwner changes. The latter is causing some issues, which I'm still debugging. Landing the former in the meantime.

(In reply to Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] from comment #2) > Splitting this patch into two parts: the operator== consting and the > exclusiveAccessOwner changes. The latter is causing some issues, which I'm > still debugging. Landing the former in the meantime. Pushed under the wrong bug number: https://bugzilla.mozilla.org/show_bug.cgi?id=1290317#c4 Ah well...

Pushed by nfitzgerald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b61757c08a5f Make JSRuntime's exclusiveAccessOwner a js::Thread::Id instead of a PRThread*; r=terrence

Backed out for asserting mIsSome in testParallelCompile.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/85272f8ab75c Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b61757c08a5fbe9df2a1af7e462650b8c5b556ba Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=33515679&repo=mozilla-inbound TEST-PASS | js/src/jit-test/tests/asm.js/testParallelCompile.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --non-writable-jitcode --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") TEST-PASS | js/src/jit-test/tests/asm.js/testParallelCompile.js | Success (code 0, args "--no-asmjs") Assertion failure: mIsSome, at /home/worker/workspace/build/src/obj-spider/dist/include/mozilla/Maybe.h:261 Exit code: -11 FAIL - asm.js/testParallelCompile.js TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/asm.js/testParallelCompile.js | Assertion failure: mIsSome, at /home/worker/workspace/build/src/obj-spider/dist/include/mozilla/Maybe.h:261 (code -11, args "") INFO exit-status : -11 INFO timed-out : False INFO stderr 2> Assertion failure: mIsSome, at /home/worker/workspace/build/src/obj-spider/dist/include/mozilla/Maybe.h:261 Assertion failure: mIsSome, at /home/worker/workspace/build/src/obj-spider/dist/include/mozilla/Maybe.h:261 Exit code: -11 FAIL - asm.js/testParallelCompile.js

Ok, so these assertions that keep failing are totally bogus. We're checking some state before locking that is not synchronized by any lock. In fact, the whole *CanLock() functions are pretty useless right now. We have checks against reentrancy and wrong thread unlocking in DEBUG builds via pthreads' ERRORCHECK mode and via the AutoLockWhatever& parameters that are thread through all these functions. It would be nice to have some dynamic ordering checks against inversion, but as it stands now, these assertions we do have are hurting way more than they are helping. I'm going to remove them.

See also comment 6. PTHREAD_MUTEX_ERRORCHECK gives us this error checking against reentrancy and unlocking an unlocked lock or another thread's lock already, so it isn't needed. This also makes the *CanLock assertions no-ops. In the future, it would be nice to introduce ordering checks against inversions. I plan to do this in a follow up.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e69421db848

Comment on attachment 8779091 [details] [diff] [review] Remove JSRuntime's exclusiveAccessOwner and *CanLock assertions Review of attachment 8779091 [details] [diff] [review]: ----------------------------------------------------------------- This looks correct. That said, I'd really like to land the patch that passes the |const AutoLockForExclusiveAccess&| to the functions you're stripping of assertions either first or concurrent with this patch.

Moved a comment about why the exclusive access lock is taken in AutoTraceSession to right above AutoTraceSession's AutoLockForExclusiveAccess member declaration.

The only place that lost a proof of lock holding assertion (as opposed to ordering assertion) was js::ExclusiveContext::setCompartment.

Try push for this and others: https://treeherder.mozilla.org/#/jobs?repo=try&revision=97ba2ddf4876

Comment on attachment 8779539 [details] [diff] [review] Part 1: Thread AutoLockForExclusiveAccess params through compartment setting functions as proof of lock holding Review of attachment 8779539 [details] [diff] [review]: ----------------------------------------------------------------- Awesome!

Pushed by nfitzgerald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8ff90165cc9 Part 0: Remove JSRuntime's exclusiveAccessOwner and *CanLock assertions; r=terrence https://hg.mozilla.org/integration/mozilla-inbound/rev/eefc85446e21 Part 1: Thread AutoLockForExclusiveAccess params through compartment setting functions as proof of lock holding; r=terrence

https://hg.mozilla.org/mozilla-central/rev/f8ff90165cc9 https://hg.mozilla.org/mozilla-central/rev/eefc85446e21