Skip to content

Enable wasm workers to run under node#19322

Merged
sbc100 merged 1 commit intomainfrom
node_wasm_workers
May 10, 2023
Merged

Enable wasm workers to run under node#19322
sbc100 merged 1 commit intomainfrom
node_wasm_workers

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 9, 2023

This means we can run the tests without depending on the browser.

@sbc100 sbc100 requested review from juj and kripken May 9, 2023 22:54
@sbc100 sbc100 force-pushed the node_wasm_workers branch 2 times, most recently from fd591c3 to a814d6e Compare May 10, 2023 00:04
This means we can run the tests without depending on the browser.
@sbc100 sbc100 force-pushed the node_wasm_workers branch from a814d6e to 908cea2 Compare May 10, 2023 01:03
@sbc100
Copy link
Collaborator Author

sbc100 commented May 10, 2023

Will wait for @juj for final approval

@sbc100
Copy link
Collaborator Author

sbc100 commented May 10, 2023

Actually I'm going to land this now since it seems fairly straight forward, and I could use this for debugging some other PRs. If the onmessage part of the change is controversial at all we can address that in a followup.

@sbc100 sbc100 merged commit ca1d374 into main May 10, 2023
@sbc100 sbc100 deleted the node_wasm_workers branch May 10, 2023 17:51
@kleisauke
Copy link
Collaborator

Interesting, I remembered that more changes were needed to fix Node.js compatibility, see e.g. PR #18201 (I should have split that PR back then, sorry).

@sbc100
Copy link
Collaborator Author

sbc100 commented May 10, 2023

This change is minimal.. just enough to get the first few tests passing. There will more changes needed I'm sure to get the rest of them running.

@dschuff
Copy link
Member

dschuff commented May 11, 2023

This seems to be causing a failure under LSan: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8781449586792126465/+/u/Emscripten_testsuite__LSan_/stdout

Aborted(Assertion failed: stackSize > tlsSize, at: /emsdk/emscripten/system/lib/wasm_worker/library_wasm_worker.c,47,emscripten_create_wasm_worker)
/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:142
   throw ex;
   ^

RuntimeError: Aborted(Assertion failed: stackSize > tlsSize, at: /emsdk/emscripten/system/lib/wasm_worker/library_wasm_worker.c,47,emscripten_create_wasm_worker)
    at abort (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:631:10)
    at ___assert_fail (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:3913:2)
    at emscripten_create_wasm_worker (<anonymous>:wasm-function[212]:0x6b28)
    at emscripten_malloc_wasm_worker (<anonymous>:wasm-function[213]:0x6b92)
    at __original_main (<anonymous>:wasm-function[46]:0xf76)
    at main (<anonymous>:wasm-function[47]:0xfb1)
    at /b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:657:20
    at callMain (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4912:13)
    at doRun (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4951:21)
    at run (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4963:3)
    at runCaller (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4901:18)
    at removeRunDependency (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:618:4)
    at /b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:889:44
    at Object.loadWasmModuleToAllWorkers (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:3753:3)
    at receiveInstance (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:889:11)
    at receiveInstantiationResult (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:897:3)
Thrown at:
    at abort (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:631:10)
    at ___assert_fail (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:3913:2)
    at emscripten_create_wasm_worker (:1:27433)
    at emscripten_malloc_wasm_worker (:1:27539)
    at __original_main (:1:3959)
    at main (:1:4018)
    at /b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:657:20
    at callMain (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4912:13)
    at doRun (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4951:21)
    at run (/b/s/w/ir/x/t/emtest_ttbbjo15/emscripten_test_lsan_0bdudi02/malloc_wasm_worker.js:4963:3)
    ```

@dschuff
Copy link
Member

dschuff commented May 11, 2023

Actually I guess the problem is not new, it's just that we weren't running this test outside of the browser before, right?

@sbc100
Copy link
Collaborator Author

sbc100 commented May 11, 2023

Right, old bug newly visible I would guess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants