Skip to content

Fix the WASM Worker cannot start in node.js environment #20188

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

Merged
merged 2 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions emscripten.py
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,7 @@ def create_pointer_conversion_wrappers(metadata):
'__main_argc_argv': '__PP',
'emscripten_stack_set_limits': '_pp',
'__set_stack_limits': '_pp',
'__set_thread_state': '_p___',
'__cxa_can_catch': '_ppp',
'__cxa_increment_exception_refcount': '_p',
'__cxa_decrement_exception_refcount': '_p',
Expand Down
15 changes: 14 additions & 1 deletion src/library_wasm_worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ addToLibrary({
$_wasmWorkerRunPostMessage: (e) => {
// '_wsc' is short for 'wasm call', trying to use an identifier name that
// will never conflict with user code
let data = e.data, wasmCall = data['_wsc'];
#if ENVIRONMENT_MAY_BE_NODE
// In Node.js environment, message event 'e' containing the actual data sent,
// while in the browser environment it's contained by 'e.data'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this the case, though? If it arrives from a Node.js or Web API then that means this workaround is necessary, but if not, then if it arrives from our own code, can we send data in the same format?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, reading that again, I think you're saying it's the first? We send data the same way, but Node and the Web end up providing it differently? If so then lgtm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, reading that again, I think you're saying it's the first? We send data the same way, but Node and the Web end up providing it differently? If so then lgtm.

yeah, you are right! 👍

let data = ENVIRONMENT_IS_NODE ? e : e.data;
#else
let data = e.data;
#endif
let wasmCall = data['_wsc'];
wasmCall && getWasmTableEntry(wasmCall)(...data['x']);
},

Expand Down Expand Up @@ -155,6 +162,12 @@ if (ENVIRONMENT_IS_WASM_WORKER) {
#endif
'sb': stackLowestAddress, // sb = stack bottom (lowest stack address, SP points at this when stack is full)
'sz': stackSize, // sz = stack size
#if USE_OFFSET_CONVERTER
'wasmOffsetData': wasmOffsetConverter,
#endif
#if LOAD_SOURCE_MAP
'wasmSourceMapData': wasmSourceMap,
#endif
});
worker.onmessage = _wasmWorkerRunPostMessage;
return _wasmWorkersID++;
Expand Down
16 changes: 16 additions & 0 deletions src/parseTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,18 @@ function runIfMainThread(text) {
}
}

function runIfWorkerThread(text) {
if (WASM_WORKERS && PTHREADS) {
return `if (ENVIRONMENT_IS_WASM_WORKER || ENVIRONMENT_IS_PTHREAD) { ${text} }`;
} else if (WASM_WORKERS) {
return `if (ENVIRONMENT_IS_WASM_WORKER) { ${text} }`;
} else if (PTHREADS) {
return `if (ENVIRONMENT_IS_PTHREAD) { ${text} }`;
} else {
return '';
}
}

function expectToReceiveOnModule(name) {
return INCOMING_MODULE_JS_API.has(name);
}
Expand Down Expand Up @@ -1031,3 +1043,7 @@ function getPerformanceNow() {
return 'performance.now';
}
}

function implicitSelf() {
return ENVIRONMENT.includes('node') ? 'self.' : '';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the environment includes both node and web and we run the result on the web? i.e. isn't choosing to use self. or not here at compile time too early.. don't we need to decide at runtime maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the web environment, it can run with or without the self. prefix, but in the node environment, it must have the self. prefix to run.
So I think the above code is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. It looks like we have the same pattern in src/worker.js but we simply always use self there.

I wonder if we could find a way to avoid the self. completely by using defineProperty on the global object under node? Maybe that would be more code overall?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this issue is the same as the issue below, it can create a separate PR after merging this to deal with these two issues together.

}
18 changes: 7 additions & 11 deletions src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ function instantiateSync(file, info) {
}
#endif

#if PTHREADS && (LOAD_SOURCE_MAP || USE_OFFSET_CONVERTER)
#if (PTHREADS || WASM_WORKERS) && (LOAD_SOURCE_MAP || USE_OFFSET_CONVERTER)
// When using postMessage to send an object, it is processed by the structured
// clone algorithm. The prototype, and hence methods, on that object is then
// lost. This function adds back the lost prototype. This does not work with
Expand Down Expand Up @@ -1082,22 +1082,18 @@ function createWasm() {
// path.
if (Module['instantiateWasm']) {

#if USE_OFFSET_CONVERTER && PTHREADS
if (ENVIRONMENT_IS_PTHREAD) {
#if USE_OFFSET_CONVERTER
#if ASSERTIONS
assert(Module['wasmOffsetData'], 'wasmOffsetData not found on Module object');
{{{ runIfWorkerThread("assert(Module['wasmOffsetData'], 'wasmOffsetData not found on Module object');") }}}
#endif
wasmOffsetConverter = resetPrototype(WasmOffsetConverter, Module['wasmOffsetData']);
}
{{{ runIfWorkerThread("wasmOffsetConverter = resetPrototype(WasmOffsetConverter, Module['wasmOffsetData']);") }}}
#endif

#if LOAD_SOURCE_MAP && PTHREADS
if (ENVIRONMENT_IS_PTHREAD) {
#if LOAD_SOURCE_MAP
#if ASSERTIONS
assert(Module['wasmSourceMapData'], 'wasmSourceMapData not found on Module object');
{{{ runIfWorkerThread("assert(Module['wasmSourceMapData'], 'wasmSourceMapData not found on Module object');") }}}
#endif
wasmSourceMap = resetPrototype(WasmSourceMap, Module['wasmSourceMapData']);
}
{{{ runIfWorkerThread("wasmSourceMap = resetPrototype(WasmSourceMap, Module['wasmSourceMapData']);") }}}
#endif

try {
Expand Down
39 changes: 36 additions & 3 deletions src/wasm_worker.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,51 @@
// N.B. The contents of this file are duplicated in src/library_wasm_worker.js
// in variable "_wasmWorkerBlobUrl" (where the contents are pre-minified) If
// doing any changes to this file, be sure to update the contents there too.
onmessage = function(d) {

'use strict';

#if ENVIRONMENT_MAY_BE_NODE
// Node.js support
var ENVIRONMENT_IS_NODE = typeof process == 'object' && typeof process.versions == 'object' && typeof process.versions.node == 'string';
if (ENVIRONMENT_IS_NODE) {
// Create as web-worker-like an environment as we can.

var nodeWorkerThreads = require('worker_threads');

var parentPort = nodeWorkerThreads.parentPort;

parentPort.on('message', (data) => typeof onmessage === "function" && onmessage({ data: data }));

var fs = require('fs');

Object.assign(global, {
self: global,
require,
location: {
href: __filename
},
Worker: nodeWorkerThreads.Worker,
importScripts: (f) => (0, eval)(fs.readFileSync(f, 'utf8') + '//# sourceURL=' + f),
postMessage: (msg) => parentPort.postMessage(msg),
performance: global.performance || { now: Date.now },
addEventListener: (name, handler) => parentPort.on(name, handler),
removeEventListener: (name, handler) => parentPort.off(name, handler),
});
}
#endif // ENVIRONMENT_MAY_BE_NODE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like duplication of the code in src/worker.js .. is it worth factoring out into a separate file that can be included in both places. Something like src/node_webworker_polyfill.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this issue is the same as the issue above, can create a separate PR after merging this to deal with these two issues together.


{{{ implicitSelf() }}}onmessage = function(d) {
// The first message sent to the Worker is always the bootstrap message.
// Drop this message listener, it served its purpose of bootstrapping
// the Wasm Module load, and is no longer needed. Let user code register
// any desired message handlers from now on.
onmessage = null;
{{{ implicitSelf() }}}onmessage = null;
d = d.data;
#if !MODULARIZE
self.{{{ EXPORT_NAME }}} = d;
#endif
#if !MINIMAL_RUNTIME
d['instantiateWasm'] = (info, receiveInstance) => { var instance = new WebAssembly.Instance(d['wasm'], info); receiveInstance(instance, d['wasm']); return instance.exports; }
d['instantiateWasm'] = (info, receiveInstance) => { var instance = new WebAssembly.Instance(d['wasm'], info); return receiveInstance(instance, d['wasm']); }
#endif
importScripts(d.js);
#if MODULARIZE
Expand Down
11 changes: 9 additions & 2 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -9912,16 +9912,23 @@ def test_emscripten_async_load_script(self):
self.run_process([FILE_PACKAGER, 'test.data', '--preload', 'file1.txt', 'file2.txt', '--from-emcc', '--js-output=script2.js'])
self.do_runf(test_file('test_emscripten_async_load_script.c'), emcc_args=['-sFORCE_FILESYSTEM'])

def prep_wasm_worker_in_node(self):
# Auto exit after 3 seconds in Nodejs environment to get WASM Worker stdout
self.add_pre_run("setTimeout(()=>process.exit(), 3000);")

@node_pthreads
def test_wasm_worker_hello(self):
self.do_runf(test_file('wasm_worker/hello_wasm_worker.c'), emcc_args=['-sWASM_WORKERS'])
self.prep_wasm_worker_in_node()
self.do_run_in_out_file_test(test_file('wasm_worker/hello_wasm_worker.c'), emcc_args=['-sWASM_WORKERS'])

@node_pthreads
def test_wasm_worker_malloc(self):
self.do_runf(test_file('wasm_worker/malloc_wasm_worker.c'), emcc_args=['-sWASM_WORKERS'])
self.prep_wasm_worker_in_node()
self.do_run_in_out_file_test(test_file('wasm_worker/malloc_wasm_worker.c'), emcc_args=['-sWASM_WORKERS'])

@node_pthreads
def test_wasm_worker_wait_async(self):
self.prep_wasm_worker_in_node()
self.do_runf(test_file('wasm_worker/wait_async.c'), emcc_args=['-sWASM_WORKERS'])


Expand Down
6 changes: 3 additions & 3 deletions test/wasm_worker/hello_wasm_worker.c
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#include <assert.h>
#include <emscripten.h>
#include <emscripten/wasm_worker.h>
#include <stdio.h>
#include <assert.h>

// This is the code example in site/source/docs/api_reference/wasm_workers.rst

void run_in_worker()
{
printf("Hello from wasm worker!\n");
emscripten_console_log("Hello from wasm worker!\n");
#ifdef REPORT_RESULT
REPORT_RESULT(0);
#endif
Expand Down
1 change: 1 addition & 0 deletions test/wasm_worker/hello_wasm_worker.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello from wasm worker!
1 change: 1 addition & 0 deletions test/wasm_worker/malloc_wasm_worker.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello from wasm worker!