emscripten-core / emscripten-core/emscripten

Proxying C++ API doesn't run lambda's destructor on the target thread

Open
#20,610 1 comment 0 reactions 0 assignees View on GitHub
multithreading
Dominant language
C++
Stars
27.6k
Forks
3.6k
Avg merge
1d 1h
Merged PRs (30d)
105

Description

Some types are sensitive to being used and destructed on the same thread they came from. One example of this is `emscripten::val` that represents JS values. Its destructor calls into JS to decrease reference counter of a JS value, but the value stored behing the handle would be different or not exist at all if invoked on the wrong thread.

Proxying currently invokes lambda on the target thread, but doesn't destruct it there, which means that even if you move a value into the lambda, it still won't be destructed on the correct thread.

Example:

```cpp
#include
#include
#include
#include
#include

using namespace emscripten;

int main() {
ProxyingQueue queue;

val v;
queue.proxySync(emscripten_main_runtime_thread_id(), [&] {
// create a value on the main thread but store on the current one
new (&v) val(42);
});

queue.proxySync(emscripten_main_runtime_thread_id(), [v = std::move(v)] {
// move the value back to the main thread to be used & destroyed
val::global("console").call("log", v);
});
// val should have been freed by now by the target thread
}
```

```
> ./emcc temp.cpp -o temp.js -lembind -s PROXY_TO_PTHREAD -g2 -pthread
> node temp.js
Aborted(Assertion failed: pthread_equal(thread, pthread_self()) && "val accessed from wrong thread", at: C:\Users\me\Documents\emscripten\cache\sysroot/include\emscripten/val.h,391,as_handle)
C:\Users\me\Documents\emscripten\temp.js:147
throw ex;
^

RuntimeError: Aborted(Assertion failed: pthread_equal(thread, pthread_self()) && "val accessed from wrong thread", at: C:\Users\me\Documents\emscripten\cache\sysroot/include\emscripten/val.h,391,as_handle)
at abort (C:\Users\me\Documents\emscripten\temp.js:753:11)
at ___assert_fail (C:\Users\me\Documents\emscripten\temp.js:1760:7)
at temp.wasm.emscripten::val::as_handle() const (wasm://wasm/temp.wasm-00073e76:wasm-function[57]:0x13f5)
at temp.wasm.emscripten::val::~val() (wasm://wasm/temp.wasm-00073e76:wasm-function[50]:0x107e)
at temp.wasm.emscripten::val::operator=(emscripten::val&&) & (wasm://wasm/temp.wasm-00073e76:wasm-function[167]:0x3833)
at temp.wasm.main::$_0::operator()() const (wasm://wasm/temp.wasm-00073e76:wasm-function[165]:0x3759)
at temp.wasm.decltype(std::declval()()) std::__2::__invoke[abi:v160006](main::$_0&) (wasm://wasm/temp.wasm-00073e76:wasm-function[163]:0x36ba)
at temp.wasm.void std::__2::__invoke_void_return_wrapper::__call(main::$_0&) (wasm://wasm/temp.wasm-00073e76:wasm-function[162]:0x367f)
at temp.wasm.std::__2::__function::__alloc_func, void ()>::operator()[abi:v160006]() (wasm://wasm/temp.wasm-00073e76:wasm-function[99]:0x23ab)
at temp.wasm.std::__2::__function::__func, void ()>::operator()() (wasm://wasm/temp.wasm-00073e76:wasm-function[98]:0x236a)

Node.js v20.7.0
```

I think `proxySync` needs either a handling similar to `proxyAsync` or, to avoid heap allocation, move it into something like `std::optional` to destruct it on the target thread and avoid 2nd destructor running on the main thread.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.