Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WASMFS JS proxy thread assertion too strict #23636

Open
JoeOsborn opened this issue Feb 9, 2025 · 2 comments
Open

WASMFS JS proxy thread assertion too strict #23636

JoeOsborn opened this issue Feb 9, 2025 · 2 comments

Comments

@JoeOsborn
Copy link
Contributor

Right now, JS code in Module.onRuntimeInitialized can’t create certain WASMFS backends before main runs, which is something that’s possible in the JS filesystem and possible under wasmfs with JSPI. This is because e.g. fetchfs and opfs backends (as well as thread_util.h’s ProxyWorker) currently assert at construction time that they are either not on the main thread or they are running with JSPI.

The motivation behind these assertions is that the main thread may not be able to spawn a thread in a fresh worker without causing a deadlock, but sometimes it’s possible to do this safely (either to succeed or at least to fail if a worker is not available). Specifically, if the worker pool has a free unused worker, spawning a thread will definitely succeed.

While there is currently no api the C code could use to detect whether a worker is free, when run under PTHREAD_POOL_STRICT > 0 the worker fetching code will throw a warning (if strict = 1) or error out (if strict = 2) in the condition where a deadlock could happen by spawning a new worker from the main thread. I therefore think the assertions should be conditionally compiled in only when PTHREAD_POOL_STRICT is 0, and the overly strict condition now amounts to a bug.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 9, 2025

I would go one step further and say that it should be possible to make this work even without any workers in the pool. As long as the main thread is returning the to event loop and during async work during startup it should be possible to start new workers just fine.

It should be possible to do any amount of async work in onRuntimeInitialized, including starting new threads, as long you don't try to block on the new threads work before running the event loop.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 9, 2025

@tlively

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants