`wasm_thread` leaks thread stacks

[BGR360]

When running wasm-bindgen workers that use shared memory, you need to call <wasm module>.__wbindgen_thread_destroy() from JavaScript in order to deallocate the stack for the thread, otherwise it leaks. This is a problem for applications that want to use wasm_thread to spawn many threads over a period of time rather than make a static thread pool.

This is mentioned briefly in the wasm-bindgen parallel raytracing example (emphasis my own):

There is no standard notion of a "thread". For example the standard library has no viable route to implement the std::thread module. As a consequence there is no concept of thread exit and TLS destructors will never run. We do expose a helper, __wbindgen_thread_destroy, that deallocates the thread stack and TLS. If you invoke it, it must be the last function you invoke from the Wasm module for a given thread.

Some more details on this can be found in the wasm-bindgen source code.

A practical example is shown here:

  // Clean up thread resources. Depending on what you're doing with the thread, this might
  // not be what you want. (For example, if the thread spawned some javascript tasks
  // and exited, this is going to cancel those tasks.) But if you're using threads in the
  // usual native way (where you spin one up to do some work until it finisheds) then
  // you'll want to clean up the thread's resources.

  // Free memory (stack, thread-locals) held (in the wasm linear memory) by the thread.
  init.__wbindgen_thread_destroy();
  // Tell the browser to stop the thread.
  close();

I think actually that the call to close() should come before the call to __wbindgen_thread_destroy(), that way it's guaranteed that there are no outstanding JavaScript tasks that may execute code in the Wasm module before/during the call to __wbindgen_thread_destroy(). Or maybe that's not possible?

[AlexanderSchuetz97]

Is this still a problem?

Its not a big problem for me because wasm threads take "forever" to start compared to OS threads, so pooling the threads makes sense even for pretty trivial use cases. But it certainly is one more reason for me to consider using a thread pool.

[9SMTM6]

@AlexanderSchuetz97 I expect that this is still and will continue to be a problem.

That's because its fundamentally difficult to solve from the side of this library, AFAIK. The comment in the quoted source code explains it:

Clean up thread resources. Depending on what you're doing with the thread, this might
not be what you want. (For example, if the thread spawned some javascript tasks
and exited, this is going to cancel those tasks.) But if you're using threads in the
usual native way (where you spin one up to do some work until it finisheds) then
you'll want to clean up the thread's resources.

While the approach to just always invoke the cleanup would likely do the right thing most times, it won't always, and the current behavior is consistent with the way Rust (refer e.g. to the removal of scoped threads in Rust 1.0), and it allows for more flexibility.

However, while I think the current behavior may be the best, at the very least this should be better documented.