`_emscripten_init_main_thread_js` should support `can_block=ENVIRONMENT_IS_WORKER`

[olokobayusuf]

Working with pthreads and web workers, Emscripten's check for can_block should respect ENVIRONMENT_IS_WORKER in addition to !ENVIRONMENT_IS_WEB.

emscripten/src/lib/libpthread.js

Lines 708 to 719 in bee9a1c

	__emscripten_thread_init(
	tb,
	/*is_main=*/!ENVIRONMENT_IS_WORKER,
	/*is_runtime=*/1,
	/*can_block=*/!ENVIRONMENT_IS_WEB,
	/*default_stacksize=*/{{{ DEFAULT_PTHREAD_STACK_SIZE }}},
	#if PTHREADS_PROFILING
	/*start_profiling=*/true,
	#else
	/*start_profiling=*/false,
	#endif
	);

[olokobayusuf]

I'm happy to submit a PR with the fix, it's a one-liner:

/*can_block=*/!ENVIRONMENT_IS_WEB || ENVIRONMENT_IS_WORKER,

[sbc100]

Hmm, I guess this depends if ENVIRONMENT_IS_WEB is supposed to be true on web workers or not.

It looks like we have two different pats for defining it that disagree on this point:

$ git grep ENVIRONMENT_IS_WEB|
...
src/shell.js:var ENVIRONMENT_IS_WEB = {{{ ENVIRONMENT[0] === 'web' }}};
src/shell.js:var ENVIRONMENT_IS_WEB = !!globalThis.window;
...

If the first codepath ENVIRONMENT_IS_WEB is true for the main thread and for workers.

In the second codepath its only true on the main thread since globalThis.window will be undefined.

This patch would only be needed in the code case I think?

[sbc100]

i.e. should ENVIRONMENT_IS_WEB and ENVIRONMENT_IS_WORKER be mutually exclusive or not?

@juj @kripken @dschuff WDYT?

[sbc100]

In any case it would be good to add a test for this to out existing tests for "running emscripten program in a worker".. I see we have test_hello_world_worker in test_browser.py but there may be others.

[olokobayusuf]

From a purely design perspective, it's much cleaner to have ENVIRONMENT_IS_WEB be mutually exclusive with ENVIRONMENT_IS_WORKER. You can then compose these if you need (worker + web), as opposed to doing exclusions to detect workers only.

[sbc100]

So I guess then we should fix the line src/shell.js:var ENVIRONMENT_IS_WEB = {{{ ENVIRONMENT[0] === 'web' }}}; which I assume is that one that your hitting?

i.e. so you see var ENVIRONMENT_IS_WEB = true in your generated code?

[olokobayusuf]

So I guess then we should fix the line src/shell.js:var ENVIRONMENT_IS_WEB = {{{ ENVIRONMENT[0] === 'web' }}}; which I assume is that one that your hitting?

i.e. so you see var ENVIRONMENT_IS_WEB = true in your generated code?

Correct. Basically, we are seeing that in a web worker, ENVIRONMENT_IS_WEB == true which means can_block == false. The problem here is that emscripten_futex_wait tries to use a busy-wait fallback (intended for the main browser thread where Atomics.wait isn't allowed). But then the busy wait asserts emscripten_is_main_browser_thread(), and since we're in a web worker, this fails and the program aborts.

[sbc100]

Hmm.. I tried to repro this with -sENVIRONMENT=web but failed. Are you maybe building with just -sENVIRONMENT=worker?

[sbc100]

If so can you use -sENVIRONMENT=web to fix this? (You don't need to explictly include worker here since -pthread will add that for you)

[sbc100]

I'm seeing var ENVIRONMENT_IS_WEB = !!globalThis.window; which should be false on Workers.

[olokobayusuf]

Hmm.. I tried to repro this with -sENVIRONMENT=web but failed. Are you maybe building with just -sENVIRONMENT=worker?

I am building for both the web and workers, so I don't set -sENVIRONMENT which should default to the following:

emscripten/src/settings.js

Line 670 in ecb9e39

var ENVIRONMENT = ['web', 'webview', 'worker', 'node'];

I'm seeing var ENVIRONMENT_IS_WEB = !!globalThis.window; which should be false on Workers.

Hm I'll check this out.