Bug 915735 may have broken compiling js shells and bug 947299 may have partially fixed some issues, but shells compiled with --disable-threadsafe still don't seem to work. This isn't the first failure involving --disable-threadsafe (see bug 942552), please test with this flag in the future! Setting needinfo? from Ehsan as he wrote the patch in bug 915735.

Configure parameters: CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --disable-threadsafe

Additionally, I also seem to be having trouble compiling with --enable-threadsafe on Windows (32-bit and 64-bit), though that might be related to bug 947299. (also on rev edac8cba9f78)

As you could have probably guessed form the build failure, this has nothing to do with bug 915735. $ git bisect good 3b2fc4c5d744113e6c677dd86a4df7806d02450a is the first bad commit commit 3b2fc4c5d744113e6c677dd86a4df7806d02450a Author: Jan de Mooij <jdemooij@mozilla.com> Date: Fri Dec 6 21:03:27 2013 +0100 Bug 946883 - Use NSPR thread for AsmJSMachExceptionHandler on OS X, so that it works with PosixNSPR. r=luke --HG-- extra : rebase_source : 34a82b93197c14ab237df23ceb8646499049cbf8 :040000 040000 c72d0fd18fb42d08fcf80299209f85e18e9bd8c9 42da86449ed59158aca1197433e8a116154443a0 M js

Sigh. Sorry for the mixup. Turns out that bug 946883 landed in the middle of the landing of bug 915735 and bug 947299's changesets (these broke & fixed Mac shells in some configurations), plus the build failure being in the middle rather than the end of the logfile does not help.

Luke, do you think it's ok to disable signal handlers (and therefore Odin) on OS X with --disable-threadsafe?

Yeah, just have EnsureIsSignalHandlingBroken() return 'false' in that case and Odin will turn off.

This fixes the build for me with --disable-threadsafe.

This will need to land on Aurora, I think. The regressor made the merge but not this fix.

Comment on attachment 8344675 [details] [diff] [review] Patch This patch is basically NPOTB. I'll request approval just to be sure, but we never build the browser with --disable-threadsafe so it has no effect on it. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 946883. User impact if declined: Unable to build a --disable-threadsafe shell. Testing completed (on m-c, etc.): On m-c. Risk to taking this patch (and alternatives if risky): None. String or IDL/UUID changes made by this patch: None.

NPOTB changes do not need approval.

Comment on attachment 8344675 [details] [diff] [review] Patch what Ehsan said, use a=NPTOB in your commit, thanks for checking though.

If this needs extra QA testing please remove the [qa-] whiteboard tag and add the verifyme keyword.

This is working.