You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test-shadow-realm-gc-module can crash on macOS (OOM) #50726
node:internal/process/task_queues:95
runMicrotasks();
^
TypeError: Cannot import in ShadowRealm (TypeError: A dynamic import callback was not specified.)
I ran on macOS locally, above is the error message I got with Release/node
changed the title [-]`test-shadow-realm-gc-module` crashes on macOS in debug build[/-][+]`test-shadow-realm-gc-module` can crash on macOS (OOM)[/+]on Nov 14, 2023
I checked the last two GitHub action runs on the main branch and I didn't find this failure on the macOS build (https://github.com/nodejs/node/actions/runs/6859065494/job/18650800024 failed for a different test). And I didn't reproduce the failure with 1000 runs locally. Does this problem constantly reproduce with your local build?
I think it's probably caused by the limit (20MB) being a bit too small for --node-builtin-modules-path which reads the JS sources from disks and creates new strings for every realm, instead of what we normally do (using externalized strings shared across the realms). And then the test is running the allocation in a hot loop so the GC cannot keep up. Awaiting a promisified setTimeout in the loop to give the GC some breathing room fixes it locally for me.
I am now seeing test-shadow-realm-gc flakes in V8's Node.js integration test CI too (in different unrelated CLs). We should at least try to get #50735 landed quickly. If that's still not enough to fix it we should mark it so to avoid spreading this flake to V8.
Activity
MrJithil commentedon Nov 14, 2023
I ran on macOS locally, above is the error message I got with Release/node
[-]`test-shadow-realm-gc-module` crashes on macOS in debug build[/-][+]`test-shadow-realm-gc-module` can crash on macOS (OOM)[/+]targos commentedon Nov 14, 2023
I just tried with the release build and I get the same OOM error. I updated the title.
targos commentedon Nov 14, 2023
/cc @legendecas who added the test in #48655
legendecas commentedon Nov 14, 2023
I checked the last two GitHub action runs on the main branch and I didn't find this failure on the macOS build (https://github.com/nodejs/node/actions/runs/6859065494/job/18650800024 failed for a different test). And I didn't reproduce the failure with 1000 runs locally. Does this problem constantly reproduce with your local build?
Stress test: https://ci.nodejs.org/job/node-stress-single-test/464
targos commentedon Nov 14, 2023
I am not close to my computer now but it reproduced consistently (I tried to run it between 5 and 10 times )
MrJithil commentedon Nov 14, 2023
With the latest merge? I took the latest and now unable to reproduce
for i in {1..100}; do ./node --experimental-shadow-realm --max-old-space-size=20 test/parallel/test-shadow-realm-gc-module.js; donetargos commentedon Nov 14, 2023
I have a arm64 Mac. Maybe it's platform specific
MrJithil commentedon Nov 14, 2023
I am using M1 Pro chip. Is there any customisations you did like node max heap size?
targos commentedon Nov 14, 2023
I always build with
./configure --ninja --node-builtin-modules-path=$(pwd)in case that's relevant.MrJithil commentedon Nov 14, 2023
Let me check.
marco-ippolito commentedon Nov 14, 2023
I've seen this failure in CI on different platforms, https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel8-s390x/40888/console
legendecas commentedon Nov 14, 2023
I can confirm this can be reproduced with the configure flag
--node-builtin-modules-path, in either the release build or the debug build.joyeecheung commentedon Nov 14, 2023
I think it's probably caused by the limit (20MB) being a bit too small for
--node-builtin-modules-pathwhich reads the JS sources from disks and creates new strings for every realm, instead of what we normally do (using externalized strings shared across the realms). And then the test is running the allocation in a hot loop so the GC cannot keep up. Awaiting a promisified setTimeout in the loop to give the GC some breathing room fixes it locally for me.joyeecheung commentedon Nov 17, 2023
I am now seeing test-shadow-realm-gc flakes in V8's Node.js integration test CI too (in different unrelated CLs). We should at least try to get #50735 landed quickly. If that's still not enough to fix it we should mark it so to avoid spreading this flake to V8.
test: give more time to GC in test-shadow-realm-gc-*
test: give more time to GC in test-shadow-realm-gc-*
test: give more time to GC in test-shadow-realm-gc-*
targos commentedon Nov 28, 2023
Can't reproduce anymore. We can reopen if it happens again.
test: give more time to GC in test-shadow-realm-gc-*
test: give more time to GC in test-shadow-realm-gc-*