Emscripten: Embind doesn't work on pthreads

[kripken]

E.g. doing this on a pthread will fail:

    std::string str= "Hello, world!";
    auto temp = emscripten::val(str);

Error:

exception thrown: BindingError: _emval_take_value has unknown type NSt3__212basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE,

[kripken]

cc @chadaustin @brion

[kripken]

Example full testcase:

#include <string>
#include <emscripten/bind.h>

int main() {
    std::string str= "Hello, world!";
    auto temp = emscripten::val(str);
}

Built with emcc a.cpp -o a.html -s USE_PTHREADS=1 -s PTHREAD_POOL_SIZE=1 -s PROXY_TO_PTHREAD=1 --bind -std=c++11

[brion]

I think there's two ways to go with this:
a) re-run bindings setup on each thread launch, and leave it up to the calling code to ensure that objects are only used on the thread they are registered on
b) forbid use of bindings on anything but the main web thread, with an explicit error

I think a) will be very error-prone; I've already seen issue reports with people trying to stash a val in a variable on the main thread and pick it up on a worker which fails. Though it could be useful for people who are explicitly working with JS in the worker context.

[chadaustin]

Is this wasm-only? How do pthreads work in Emscripten? (I haven't looked closely at this space since 2015!) Do pthreads not share globals in Emscripten?

[art926]

Brion, we need this feature for our project. And we are fine with option a) as far, as there is still an ability to send emscripten::val through postMessage parameter from a worker to the main thread as a result of workers job (even though it gets completely copied in memory, still worth it in our case).

[brion]

@chadaustin should be the same for wasm and JS, though due to the overhead of reloading JS in every worker thread I suspect wasm will be the primary threaded target for most folks.

Each pthread executes in the context of a Web Worker which has its own JavaScript context, so the only shared state is the SharedArrayBuffer used as the heap or Wasm memory. This means anything we stuff into JS state (such as all the embind type definitions and the handles) in one thread won't be visible in any of the others. You can transfer objects of some types explicitly from thread to thread over postMessage but you have to jump out to JS to do that, and it's all very scary. :)

[brion]

@art926 yeah, seems like there's enough scenarios where it'd be useful I wouldn't want to block it completely. Might want to make sure the failure modes are better, like being able to detect usage of a handle from another thread with a pointer tag or something and throwing a clearer error?

[art926]

@brion yeah, being able to detect usage of a handle from another thread and throwing a clear error sounds good. Btw, since you mentioned SharedArrayBuffer that's available for all thread, is there a way to construct emscripten::val there, in a heap, and use in other thread? (sorry, not familiar with the internal implementation of emscripten classes).

[art926]

@brion also, currently we are using non-documented emscripten_async_run_in_main_runtime_thread() and emscripten_async_queue_on_thread() in order to schedule a call on a specific thread and send some parameters. If those functions could accept emscripten::val as a parameter and that way send it (copy?) to another thread, it would be super useful in case of a) approach.

[brion]

@art926 unfortunately the SharedArrayBuffer can only contain raw data, you can't place JS objects in one.

I like the idea of a helper function for postMessage-transferring a JS object with an API that takes val... Definitely would simplify things. Probably can build that on top of a basic infrastructure for letting each thread work with embind on its own, then add a transfer helper function.

[kripken]

Another issue is that some of the embind code appears to not be threadsafe, e.g. the string fromWireType may modify the string in place,

https://github.com/emscripten-core/emscripten/blob/incoming/src/embind/embind.js#L634

[art926]

Did anyone have a chance to look into that more? Our project would really benefit from moving some emscripten::val construction to a thread.

[kripken]

8783 has a PR for option (a) from https://github.com/emscripten-core/emscripten/issues/8299#issuecomment-473462400 Feedback welcome. I'm not very familiar with embind, so maybe I got something wrong, but the test passes :)

That PR does not address the other issues as mentioned earlier, of having a nice warning/error when a user tries to use a JS object across threads, and doesn't help with the possible thread-safety issues, need to understand those better.

[chadaustin]

Another issue is that some of the embind code appears to not be threadsafe, e.g. the string fromWireType may modify the string in place,

https://github.com/emscripten-core/emscripten/blob/incoming/src/embind/embind.js#L634

I think that's safe because toWireType on the C++ side for strings allocates new memory every time.

[kripken]

Noting here that the above PR only helps with the builtin types, which I guess means that custom types will still hit the error message. A possible fix is described by @chadaustin there https://github.com/emscripten-core/emscripten/pull/8783#issuecomment-501438131

[VirtualTim]

This is a slight tangent, but webidl_binder.py also doesn't fully work with threads. I mean, it works well enough if you're careful, but if you do something like creating an object on one thread, and do something like Module.wrapPointer(object.ptr, Module.SomeClass) on another it will return a new SomeClass that has the same pointer as object.ptr.

Basically it's not thread safe.

Internally it uses Module.SomeClass.__cache__ to store objects, but that cache isn't shared across threads.

[kripken]

I think that is unavoidable, though, since JS objects can't be shared across threads? Just the compiled code for them can use shared memory, but not JS objects themselves. Perhaps we need to document this better? Or do you think there is something we can improve?

[VirtualTim]

Yeah I don't know. I'd like to be able to improve it, but web workers just don't seem to work the way I want them to. So I think it's impossible. Maybe it's just because I'm used to thinking about this sort of stuff in the context of native code, and that means that I'm heading about this sort of stuff the wrong way.

[stale[bot]]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.