`Array.fill` is uninterruptible

[kirjavascript]

Running this

let ivm = require('isolated-vm');
let isolate = new ivm.Isolate({ memoryLimit: 128 });
let context = isolate.createContextSync();
isolate.compileScriptSync(`[...'.'.repeat(1e9)]; undefined`).runSync(context);
console.log('this is never printed');

Gives you

<--- Last few GCs --->

[10166:0x5601c5f1fc10]        3 ms: Mark-sweep 0.9 (4.7) -> 0.9 (3.7) MB, 0.6 / 0.0 ms  (average mu = 0.014, current mu = 0.014) allocation failure GC in old space requested
[10166:0x5601c5f1fc10]        4 ms: Mark-sweep 0.9 (3.7) -> 0.9 (3.7) MB, 0.6 / 0.0 ms  (average mu = 0.016, current mu = 0.017) last resort GC in old space requested
[10166:0x5601c5f1fc10]        4 ms: Mark-sweep 0.9 (3.7) -> 0.9 (3.7) MB, 0.5 / 0.0 ms  (average mu = 0.013, current mu = 0.009) last resort GC in old space requested


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x255c5b9cfc5d]
Security context: 0x0ecddc738d71 <JSObject>
    1: next [0xecddc737f31](this=0x0ecddc741551 <StringIterator map = 0x34e5f0e8dd31>)
    2: /* anonymous */ [0xecddc73c0c9] [<isolated-vm>:1] [bytecode=0xecddc73bff1 offset=77](this=0x0ecddc73dba1 <JSGlobal Object>)
    3: InternalFrame [pc: 0x255c5b98ba89]
    4: EntryFrame [pc: 0x255c5b985b3e]

==== Details ===============================================...

CALL_AND_RETRY_LAST
is_heap_oom = 1


<--- Heap statistics --->
total_heap_size = 3883008
total_heap_size_executable = 1048576
total_physical_size = 1027440
total_available_size = 274445584
used_heap_size = 898312
heap_size_limit = 278298526
malloced_memory = 8192
peak_malloced_memory = 24576
does_zap_garbage = 0

using v1.7.10

[laverdet]

Thanks, this is an interesting case. It's resolved in the v8-heap branch. I still have a few more issues I need to resolve and then I'll roll the fix into npm. The fix requires nodejs v10.4.0 or later.

[kirjavascript]

Just tested and seems to work. However, I found something new that also causes the crash even on the v8-heap branch;

isolate.compileScriptSync(`Object.prototype[Symbol.iterator] = function() {return{next:() => this}}; [...({})]; undefined`).runSync(context);

[laverdet]

On node v12.0.0+ the iterator case is halted correctly, without crashing the main process. It does still crash on node v10.x, though. I did some digging and the behavior changed in this commit to v8: v8/v8@1c48d52bb1ee9bb28e146c60eda08cd4afaa5745. I don't think I can work around the issue in node 10.x because it's an underlying v8 issue, but the fact that it's already fixed is good news!

[kirjavascript]

can confirm is fixed in v12.0.0+ !

[kirjavascript]

something else new that crashes with the v8-heap branch

isolate.compileScriptSync(`Array(1e9).fill('ooo').join('\\n'); undefined`).runSync(context);

tested on 12.4.0

[laverdet]

Thanks for the new report. I bisected this issue back to v8 commit v8/v8@17dd105144044469a0c24ca7ef9f53225b9b8345, and it seems that v8 is still broken today. I will have to forward this to the v8 team.

I've been out of the country for a couple of weeks now so that's why there hasn't been any updates on the original issue. Hopefully I should have the v8-heap stuff in master/npm within a week or two.

[laverdet]

2.0.0 is on npm with the fix for the spread operator. I opened a v8 issue for the Array..fill crash here Issue 9368: Array..join is uninterruptible. Until I hear back on that I would suggest injecting Mozilla's polyfill [https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/fill] before running user code.

[kirjavascript]

ah cool, this works for me for now

thanks for your efforts with these issue!