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

PROXY_TO_PTHREAD support for Embind #9847

Closed
kleisauke opened this issue Nov 15, 2019 · 9 comments
Closed

PROXY_TO_PTHREAD support for Embind #9847

kleisauke opened this issue Nov 15, 2019 · 9 comments
Labels

Comments

@kleisauke
Copy link
Collaborator

kleisauke commented Nov 15, 2019

I'm creating JavaScript bindings (using Embind) for a C/C++ library that uses a lot of threading features. The bindings are compiled with these Emscripten compiler flags:

-s USE_PTHREADS=1 -s PROXY_TO_PTHREAD=1 -s ALLOW_BLOCKING_ON_MAIN_THREAD=0

Which moves the application’s main() to a pthread and disallows to block the main browser thread.

I noticed that when an Embind wrapped function (that will eventually use pthread_join or pthread_cond_wait) is invoked on the main browser thread, an error is immediately thrown. This indicates that the wrapped function is executed on the calling thread, and not on the pthread running the original main().

Is it possible to execute these functions on the pthread running the original main()?

Minimal reproduction
// answer.cpp

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

#include <future>
#include <thread>

using namespace emscripten;

int main(int argc, char *argv[]) {
    // Ensure we don't exit the runtime, see:
    // https://github.com/emscripten-core/emscripten/issues/8202
    emscripten_exit_with_live_runtime();

    return 0;
}

int answer() {
    // An arbitrary but carefully chosen number
    return 42;
}

int answerAsync() {
    std::future<int> future = std::async(answer);
    return future.get();
}

EMSCRIPTEN_BINDINGS(my_module) {
    function("answer", &answerAsync);
}

Compile with:

# Note: we set PTHREAD_POOL_SIZE to 2, otherwise the program hangs if the number of
# active threads exceed PTHREAD_POOL_SIZE. See:
# https://github.com/emscripten-core/emscripten/issues/8988
emcc answer.cpp \
  -std=c++11 \
  --bind \
  -s USE_PTHREADS=1 \
  -s PROXY_TO_PTHREAD=1 \
  -s ALLOW_BLOCKING_ON_MAIN_THREAD=0 \
  -s PTHREAD_POOL_SIZE=2 \
  -o test.html

Run Module.answer() in the developer tools console and notice that the Blocking on the main thread is not allowed by default error is thrown.

@kripken kripken added the embind label Nov 15, 2019
@kripken
Copy link
Member

kripken commented Nov 15, 2019

This is surprising - I don't think we proxy embind calls to the main thread. Perhaps the full call stack leading to that error shows some proxying code, that could explain things? (Or maybe I've forgotten something here...)

cc @jgravelle-google

@kleisauke
Copy link
Collaborator Author

Here's the stacktrace:

test.js:9206 Uncaught RuntimeError: abort(Blocking on the main thread is not allowed by default. See https://emscripten.org/docs/porting/pthreads.html#blocking-on-the-main-browser-thread) at Error
    at jsStackTrace (http://localhost:5000/test.js:1932:17)
    at stackTrace (http://localhost:5000/test.js:1949:16)
    at abort (http://localhost:5000/test.js:1700:44)
    at _emscripten_check_blocking_allowed (http://localhost:5000/test.js:5926:7)
    at __pthread_cond_timedwait (wasm-function[1261]:0x3af2f)
    at std::__2::__libcpp_condvar_wait(pthread_cond_t*, pthread_mutex_t*) (wasm-function[112]:0x2c6b)
    at dynCall_iii (wasm-function[1307]:0x3d430)
    at Module.dynCall_iii (http://localhost:5000/test.js:8988:39)
    at invoke_iii (http://localhost:5000/test.js:9181:12)
    at std::__2::condition_variable::wait(std::__2::unique_lock<std::__2::mutex>&) (wasm-function[110]:0x2bfa)
    at abort (http://localhost:5000/test.js:1706:9)
    at _emscripten_check_blocking_allowed (http://localhost:5000/test.js:5926:7)
    at __pthread_cond_timedwait (wasm-function[1261]:0x3af2f)
    at std::__2::__libcpp_condvar_wait(pthread_cond_t*, pthread_mutex_t*) (wasm-function[112]:0x2c6b)
    at dynCall_iii (wasm-function[1307]:0x3d430)
    at Module.dynCall_iii (http://localhost:5000/test.js:8988:39)
    at invoke_iii (http://localhost:5000/test.js:9181:12)
    at std::__2::condition_variable::wait(std::__2::unique_lock<std::__2::mutex>&) (wasm-function[110]:0x2bfa)
    at std::__2::__assoc_sub_state::__sub_wait(std::__2::unique_lock<std::__2::mutex>&) (wasm-function[130]:0x2fb7)
    at dynCall_vii (wasm-function[1318]:0x3d502)

Notice that it will call the C function pointer from JS on the main browser thread. I had a quick go with patching the makeDynCaller function in $embind__requireFunction, see:

concept9847.js
// concept9847.js
// build with: --js-library concept9847.js

// --js-library must go after --bind
if (!LibraryManager.library['$embind__requireFunction']) {
    throw Error("embind.js didn't run yet");
}

mergeInto(LibraryManager.library, {
    $embind__requireFunction: function (signature, rawFunction) {
        signature = readLatin1String(signature);

        function makeDynCallerProxy() {
            var args = [];
            for (var i = 1; i < signature.length; ++i) {
                args.push('a' + i);
            }

            // TODO: Make it work for other signatures as well. Perhaps something like this?:
            /*var sig;
            switch (signature) {
                case 'v':
                    sig = {{{ cDefine('EM_FUNC_SIG_V') }}};
                    break;
                case 'vi':
                    sig = {{{ cDefine('EM_FUNC_SIG_VI') }}};
                    break;
                case 'vii':
                    sig = {{{ cDefine('EM_FUNC_SIG_VII') }}};
                    break;
                case 'viii':
                    sig = {{{ cDefine('EM_FUNC_SIG_VIII') }}};
                    break;
                case 'i':
                    sig = {{{ cDefine('EM_FUNC_SIG_I') }}};
                    break;
                case 'ii':
                    sig = {{{ cDefine('EM_FUNC_SIG_II') }}};
                    break;
                case 'iii':
                    sig = {{{ cDefine('EM_FUNC_SIG_III') }}};
                    break;
                case 'iiii':
                    sig = {{{ cDefine('EM_FUNC_SIG_IIII') }}};
                    break;
                default:
                    throwBindingError("No dynCall invoker for signature: " + signature);
            }*/

            var name = 'dynCall_' + signature + '_' + rawFunction;
            var body = 'return function ' + name + '(' + args.join(', ') + ') {\n';
            // body += '    var sig = '+ sig + ';\n';
            body += '    var stackTop = stackSave();\n';
            // TODO: These parameters are hardcoded, obviously we want something smarter here.
            body += '    var varargs = stackAlloc(4);\n';
            body += '    {{{ makeSetValue('varargs', 0, 'a1', 'i32')}}};\n';
            // TODO: 5414848 = is the result of pthread_self() in the proxied main (`__emscripten_thread_main`)
            body += '     _emscripten_async_queue_on_thread_(5414848, {{{ cDefine('EM_FUNC_SIG_II') }}}, rawFunction, 0, varargs);\n';
            body += '    stackRestore(stackTop);\n';
            // TODO: Wait with `emscripten_wait_for_call_v` and return result.
            body    += '};\n';

            return (new Function('rawFunction', body))(rawFunction);
        }

        var fp;
        if (Module['FUNCTION_TABLE_' + signature] !== undefined) {
            fp = Module['FUNCTION_TABLE_' + signature][rawFunction];
        } else if (typeof FUNCTION_TABLE !== "undefined") {
            fp = FUNCTION_TABLE[rawFunction];
        } else {
            fp = makeDynCallerProxy();
        }

        if (typeof fp !== "function") {
            throwBindingError("unknown function pointer with signature " + signature + ": " + rawFunction);
        }
        return fp;
    },
});

With this patch, answerAsync() is called on the proxied main (__emscripten_thread_main) and therefore won't block the main browser thread. Obviously it will not return any result (since I didn't wait with emscripten_wait_for_call_v) and it only works with this function signature (I've hardcoded some things).

Unfortunately, I couldn't use the handy emscripten_sync_run_in_main_thread_* methods here, since these functions will run on emscripten_main_browser_thread_id and not on the proxied main.

This could be a bit of a niche issue. Hopefully I've clarified a couple of things.

@kleisauke
Copy link
Collaborator Author

kleisauke commented Nov 19, 2019

Here is an improved workaround, which seems to work with this particular example:
https://gist.github.com/kleisauke/bb1fdcf0e282234376532de3b423a931

Unfortunately this workaround doesn't work with the bindings that I'm working on, it seems to fail on this assert when it's calling emscripten_current_thread_process_queued_calls in a worker. I'll look into the cause of this and try to make a minimal reproduction.
Edit:
Workaround has moved to a gist and the above mentioned issue has been resolved by replacing EM_ASM(noExitRuntime = true); with emscripten_exit_with_live_runtime();.

@kripken
Copy link
Member

kripken commented Nov 20, 2019

Interesting. I'm not sure what's going on here, as I see no proxying in that stack trace, weird...

So this may be an embind bug, but I don't see any proxying in the embind code.

Can you perhaps add some debug logging in the various functions, to see where it proxies from one thread to the other?

@kleisauke kleisauke changed the title --proxy-to-worker support for Embind PROXY_TO_PTHREAD support for Embind Nov 21, 2019
@kleisauke
Copy link
Collaborator Author

I updated a few comments and the issue title, since I mixed up the --proxy-to-worker and the -s PROXY_TO_PTHREAD=1 linker flags. Also the workaround is getting a bit too big, so I moved it to a separate gist:
https://gist.github.com/kleisauke/bb1fdcf0e282234376532de3b423a931

Can you perhaps add some debug logging in the various functions, to see where it proxies from one thread to the other?

Sure, which functions should I debug specifically? If you want, I can also provide the output of -s LIBRARY_DEBUG=1.

@kripken
Copy link
Member

kripken commented Nov 25, 2019

The goal would be to get a stack trace showing some embind code that calls proxying code, or at least the proxying code itself, with maybe something else calling it.

Alternatively, if you can reduce this to a simple testcase with instructions, that would help too - I find it hard to follow the "workaround" sample you provide. However, perhaps @jgravelle-google who knows more about embind can find time to look at this, as the answer may be something trivial I am missing.

@kleisauke
Copy link
Collaborator Author

I've created a minimal reproducible testcase. See commit kleisauke@240b43b.

To reproduce:

git clone --single-branch --branch testcase-9847 https://github.com/kleisauke/emscripten.git testcase-9847
cd testcase-9847
python tests/runner.py browser.test_pthread_main_thread_blocking_embind

Hope this helps.

@kleisauke
Copy link
Collaborator Author

I just came across #9910, which seems to be a much more convenient solution than moving all embind dynCalls to a separate pthread.

@kleisauke
Copy link
Collaborator Author

PROXY_TO_PTHREAD support for Embind wrapped functions is odd, so I'll close this issue for now.

The issue I had is that the error produced by the -s ALLOW_BLOCKING_ON_MAIN_THREAD=0 compiler flag always occurs, even when compiling with -s PTHREAD_POOL_SIZE=N where N > 0. See for example this test case: kleisauke@6fd7e6b.

Ideally, the error in this test case should only occur if the PTHREAD_POOL_SIZE compiler flag is omitted because it works properly (i.e. no deadlocks) when compiling with a predefined pool (#8988 might be relevant here).

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

No branches or pull requests

3 participants