Couple of flaky pthread tests on Firefox

[RReverser]

I'm commonly seeing these 5 tests in particular failing on the CI in Firefox, and working after couple of attempts:

• test_pthread_sbrk
• test_pthread_safe_stack
• test_pthread_run_script
• test_pthread_run_on_main_thread_flood
• test_pthread_run_on_main_thread

Here's an example log from recently seen failures (attaching as a file because I'm not sure about retention duration on CircleCI):
log.txt

[RReverser]

Maybe sbrk is related to #10006? It was closed as stale, but I guess it's still an issue that, for whatever reason, manifests in Firefox more frequently than elsewhere.

Doesn't explain others though.

[RReverser]

OTOH I wonder if it's something that would be accidentally fixed by my changes in #18305.

One of the things I noticed and had to do there is manually start executing messages only after Module variable has been definitely assigned. It became a pretty noticeable issue in Firefox due to my changes to postMessage scheduling in the same PR, but now I wonder if there was always a racy issue, just less noticeable before.

A bit suspiciously, that PR also passed on the first run, but it still might be all a coincidence...

[RReverser]

Although no, that should've affected only modularize builds, and those tests are not it...

[RReverser]

OTOH I wonder if it's something that would be accidentally fixed by my changes in #18305.

Just tested that theory by extracting those changes separately and, no, that doesn't help. Must be some other reason, although I don't see what's common between specifically those 5 tests.

[kleisauke]

Looking at the log, I think(?) that test_pthread_run_on_main_thread makes the whole browser unresponsive, causing subsequent tests to fail as well. Which explains that this is only a issue on Firefox, because on Chrome this test is already disabled (see commit 6ea085c).

As for a possible fix, perhaps commit 25a6cb2 in PR #13007 would fix this? Just a wild guess, it's based upon that this test makes heavily use of those helpers in <emscripten/threading.h>, which would end-up calling emscripten_wait_for_call_v() in most cases.

That PR also includes some other goodies, such as commit fd05b36. Sorry for not further upstreaming those commits, I sometimes have limited spare-time. :(

[kleisauke]

... as always, I could certainly use some help in upstreaming these patches. FWIW, I'm currently tracking this in the "Upstream patches" card at https://github.com/users/kleisauke/projects/1.

[kleisauke]

As for a possible fix, perhaps commit 25a6cb2 in PR #13007 would fix this?

Though, I think the intention of PR #15681 was to deprecate emscripten_wait_for_call_v() in favor of the proposed APIs mentioned in comment #15705 (comment).

[RReverser]

As for a possible fix, perhaps commit 25a6cb2 in PR #13007 would fix this?

Though, I think the intention of PR #15681 was to deprecate emscripten_wait_for_call_v() in favor of the proposed APIs mentioned in comment #15705 (comment).

Ok I'm not familiar with that side of the things so tagging @tlively.

[tlively]

emscripten_wait_for_call_v should be implemented in terms of the new proxying API (with the system proxying queue), so it's not inherently bad to use, but should only be used for things that act like signal handlers. Looking at the test_pthread_run_on_main_thread test, it looks like it's calling printf, which internally acquires locks. My guess is that printfs are racing, and when one acquires the lock, the system proxying queue is pumped and another starts running in the middle of the first, so the main thread is deadlocking on itself.

[RReverser]

Looks like test_pthread_proxying_cpp is also flaky, probably for similar reasons. A bit hard to tell as that test seems to combine several in one file.