Don't call _emscripten_futex_wake if not a function

[VirtualTim]

This can happen if the thread throws on initialisation. Trying to call a function that doesn't exist will throw, hiding the previously caught exception.
Fixes #8446.

[kripken]

How does this hide the previous exception? Both should be logged, I'd think?

I tend to think that we should not add code that tries to make things work if the thread failed to even initialize - then the entire state of the world is just bad, and nothing can be expected to work. There are any number of other possible problems in that case, I'd guess.

[VirtualTim]

So the code I modified is in a try-catch. The logic is basically:
If a thread throws,

1. check if the exception is not "Canceled! | SimulateInfiniteLoop | pthread_exit"
2. mark the thread as stopped,
3. restart threads waiting on this one,
4. then rethrow the caught error.

If restarting threads (3.) throws, then the previously caught error never gets re-thrown (4.).

The point of this isn't to try and keep things working, its to try and make sure that the error that gets bubbled up is the initial error, not Module._emscripten_futex_wake is not a function. The purpose of this is for developers, not for the end user.

Basically this is about making the thrown error the correct/original one.

[kripken]

I see. Ok, makes sense to improve the error for developers, I agree. How about doing something like this, though?

#if ASSERTIONS
  assert(..that thing is a function.., ".. error message that explains pthreads were not set up properly etc. " + original error message);
#endif

Putting it behind ASSERTIONS ensures it doesn't add overhead in general, and I think it's clearer as an assert (rather than an if that just skips something).

[VirtualTim]

Yeah I guess I can do that. I just thought overhead didn't really matter since it only reaches this check if something goes fatally wrong.

[kripken]

The overhead is minor, I agree, but I think it's also a matter of clarity - an assertion directly says what we expect to be true there.

[VirtualTim]

I guess, but you lose useful information like the stack trace. Instead you get the stack trace of the wrong exception. And in builds without ASSERTIONS defined the wrong error is still thrown.
I guess I could put an #if ASSERTIONS around that line? Would that be acceptable?

[sbc100]

Whichever way you do it can you make you there is comment about why _emscripten_futex_wake might end up not being a function at this point in the code?

[kripken]

Good point about the wrong stack trace being shown.

How about this: At the top of this else body, we know that this isn't an infinite loop etc., and then we want to do some changes like these Atomics and the futex stuff. How about if before the atomics, we check if we managed to set up the thread properly, that is, move your if to up here, and put it behind ASSERTIONS. So something like

#if ASSERTIONS
    if (typeof({{{ makeAsmGlobalAccessInPthread('_emscripten_futex_wake') }}}) !== "function") {
      err("could not set up thread properly");
      throw e;
   }
#endif

[VirtualTim]

Done.

[kripken]

Great, thanks! Sorry for the amount of time this took, but it's important to get it right.

Odd errors on CI, but I'm sure they can't be because of this, merging.