Is wasi_thread_exit needed?
#7
Comments
|
Interesting point regarding cleanup. Presumably |
|
Note that we could consider not supporting I'm including the say its worth supported since thread is a lot of pthread code in the wild and pthread_exit is not a rare as things like thread cancellation (which I think we may be able to avoid supported). |
Oh, great point! They're both on the libc side so both places can have the cleanup code. |
|
the current wasi-libc implementation seems to use |
Unlike many of platforms with threads/lwps, wasi doesn't have separate api for thread exit and process exit as of writing this. It only has a single `wasi_proc_exit`, which is currently used for both of process exit and thread exit by wasi-libc. Becasue of that, on wasi, exit(3) only terminates the calling thread. On the other hand, the stdio exit logic uses FFINALLOCK macro, which leaves the file locked. While it's fine on platforms where exit somehow forcibly terminates other threads soon, it can make other threads block on these locks forever on wasi. Until the situation is cleaned up, this commit makes stdio exit use normal file locking instead of the "final" variant. This change allows other threads to continue running. Hopefully those threads will exit by themselves soon. cf. WebAssembly/wasi-threads#7
|
I don't think |
|
Having another syscall for exiting the thread sounds reasonable. Also, I agree with @sbc100 about keeping the API simple and move as much of the implementation as possible to WASM code - that's what we do in |
|
@loganek do you think being able to exit a thread without unwinding the stack is critical? What do you think about simply not implementing |
|
i guess choices are: a-1. use a-2. use b-1. use b-2. use i feel a little anxios about a-1 and a-2 because they can affect non wasi-threads threading. (web worker and other embedder-managed threading) |
|
Can you explain a little more why you have concerns about |
it's just a vague concern about adding extra responsibility to existing api. there seems to be wasi users using threads without wasi-threads. |
@sbc100 I'm mainly worried about porting existing code. Whereas this won't be a problem for a new code that's written with WASM in mind and without that function, porting some of the projects might require major refactoring (which maintainers might not be happy about, so we'll end up with lots of Also, you mentioned c++'s threads - the common pattern for exiting the thread is to put everything in a Having said that, not having Just a personal note - I think both |
|
a datapoint: 4M pthread_create |
|
Thanks @yamt , I was more interested in real usecases than just a number of occurrences of a specific function on github - i.e. do we know for sure that not having a |
it isn't just a number. |
|
Based on how many programs seems to depend on |
|
I am leaning in favor of I propose we test the viability of implementing this in WAMR and Wasmtime in the first week or two of the new year and then merge it in here (and the other related PRs) once we are sure everything works. Thoughts on this? |
|
I'm leaning towards not implementing this function for now, and only add it when somebody comes with a usecase where the existence of it either is absolutely necessary or greatly simplifies implementations at scale. I was at the beginning in favour of adding the function because of number of projects that use this function. After giving more thoughts on that, I wasn't actually sure what's the actual impact of not adding the function right now. As I said earlier, it's so much easier to add a new function when we know it's really needed than removing/maintaining something we're not sure about. Having said that, I won't block adding this function now, if people here believe it's absolutely necessary to have it in the first version of wasi threads, then let's add it. I just think there's a bit of simplifications we've taken to keep the API simple for now (egnot having priorities) that we probably could postpone the other features (like thread exit) until we really need it. |
|
Ok, I'm fine with that as well; we can leave this issue open and when someone really needs it then we can just add it. |
|
IMO, it's simpler to just implement wasi_thread_exit than making possibly temporary wasi-libc changes to avoid it.
the implementation of wasi_thread_exit should be trivial because what it does is exactly a subset of wasi_proc_exit, which needs to be implemented anyway. |
The current wasi-threads has no thread-exit functionality. Thus it isn't straightforward to implement pthread_exit without leaking thread context. This commit simply disables pthread_exit for now. Also, instead of abusing `wasi_proc_exit` for thread exit, make let `wasi_thread_start` return. Note: `wasi_proc_exit` is supposed to terminate all threads in the "process", not only the calling thread. Note: Depending on the conclusion of the discussion about `wasi_thread_exit`, we might revisit this change later. References: WebAssembly/wasi-threads#7 WebAssembly/wasi-threads#17
The current wasi-threads has no thread-exit functionality. Thus it isn't straightforward to implement pthread_exit without leaking thread context. This commit simply disables pthread_exit for now. Also, instead of abusing `wasi_proc_exit` for thread exit, make `wasi_thread_start` return. Note: `wasi_proc_exit` is supposed to terminate all threads in the "process", not only the calling thread. Note: Depending on the conclusion of the discussion about `wasi_thread_exit`, we might revisit this change later. References: WebAssembly/wasi-threads#7 WebAssembly/wasi-threads#17
The current wasi-threads has no thread-exit functionality. Thus it isn't straightforward to implement pthread_exit without leaking thread context. This commit simply disables pthread_exit for now. Also, instead of abusing `wasi_proc_exit` for thread exit, make `wasi_thread_start` return. Note: `wasi_proc_exit` is supposed to terminate all threads in the "process", not only the calling thread. Note: Depending on the conclusion of the discussion about `wasi_thread_exit`, we might revisit this change later. References: WebAssembly/wasi-threads#7 WebAssembly/wasi-threads#17
|
a datapoint: cpython has |
|
@abrown @loganek I have a use case that I would like to be covered by wasi-threads that is impossible to do without I would like to implement an adapter, purely using the Component Model, which allows one WASI CLI command to invoke others and interact with them via the filesystem and standard I/O streams. This is extremely useful for porting toolchains to run in the WASI world. I am working with FPGA toolchains, which are obscure, but a more familiar example--which is also relevant here!--is gcc: the For the most part, this use case can be handled by careful component composition. The environment and arguments can be copied around easily; the clocks, filesystem, socket, and random implementations can be provided by the host and/or virtualized using WASI-Virt; and the terminal or standard streams can be virtualized by an adapter. Concurrency can be provided by making the adapter spawn a thread per launched sub-command. However, there is one API that throws a spanner in the works: At the moment, without assuming that exception handling is available (it isn't on Wasmtime and probably won't be for quite a while still), there is no way to provide If I were able to map Without
|
In a discussion yesterday with @sbc100 and @sunfishcode, we considered whether it might be necessary to add a second API call to the
wasi-threadsproposal to support early exit from threads. Since exception support is neither standardized nor widespread, it is impossible for a child thread to "early exit" and continue execution in the exported thread entry point,wasi_thread_start. This entry point can contain code responsible for important tasks, such as notifying joiners that the thread is complete (e.g., in wasi-libc), and possibly others such as cleaning up any TLS allocations.One suggestion was to implement
pthread_exitin wasi-libc using a new API call,wasi_thread_exit. But consider what happens whenwasi_thread_exitis called: it could presumably dispose of the child thread from the host side, but it would not really have access to execute the cleanup code at the end ofwas_thread_start(e.g., the joinnotifyinstructions). To properly supportpthread_exitit may be necessary to do more, e.g., exporting a second function from threaded modules,wasi_thread_stop.Any additional thoughts on this?
One side issue we discussed is that traps and calls to
exitin child threads take down the entire program. This eliminates a whole other set of problems and we should probably document this in theREADME.The text was updated successfully, but these errors were encountered: