laverdet / isolated-vm

Secure & isolated JS environments for nodejs
ISC License
2.19k stars 154 forks source link

Memory leak when isolate is passed to the Sandbox context #510

Open skvelymake opened 4 days ago

skvelymake commented 4 days ago

Overview

I suspect the Isolate referenced in itself is never garbage collected. I have even tried calling jail.deleteSync('isolate'), but without any change.

Calling isolate.dispose() of course works.

Personal Diagnostics

Please answer the following questions:

JavaScript includes a setTimeout function:

Functions are a type of primitive value in JavaScript:

Objects can be shared between isolates:

The Code

let ivm = require('isolated-vm');

async function run() {
    while (true) {
        {
            let isolate = new ivm.Isolate;
            let context = isolate.createContextSync();
            let jail = context.global;
            jail.setSync('isolate', isolate);
            isolate.compileScriptSync('new '+ function() {
                isolate.compileScriptSync('1')
            }).runSync(context);

            const memoryUsage = process.memoryUsage();
            const rssMemory = Math.round(memoryUsage.rss / 1024 / 1024);

            console.log(`RSS memory: ${rssMemory} MB`);
        }

        if (global.gc) {
            global.gc();
        }

        await new Promise(resolve => setTimeout(resolve, 10));
    }
}

run().catch(console.error);
laverdet commented 4 days ago

gc is a function provided by v8 which runs the diagnostic garbage collector in the current isolate. It would not affect the isolated-vm hosted isolate if you run it from within the nodejs isolate.

Try: node --no-node-snapshot --expose-gc test.cjs

let ivm = require('isolated-vm');

async function run() {
    while (true) {
        {
            let isolate = new ivm.Isolate;
            let context = isolate.createContextSync();
            let jail = context.global;
            jail.setSync('isolate', isolate);
            isolate.compileScriptSync(`
                isolate.compileScriptSync('1');
                // nb: This doesn't work and I cannot explain it
                // delete globalThis.isolate;
            `);
            jail.deleteSync('isolate');
            context.evalSync('gc()');
            context.release();
            gc();

            const memoryUsage = process.memoryUsage();
            const rssMemory = Math.round(memoryUsage.rss / 1024 / 1024);

            console.log(`RSS memory: ${rssMemory} MB`);
        }

        await new Promise(resolve => setTimeout(resolve, 10));
    }
}

run().catch(console.error);

It is strange that deleting the reference from inside the isolate doesn't work. Anyway, it is easy to create circular references between isolates in this manner. v8 cannot detect those. It is certainly best to dispose the isolate explicitly if you're going to give an isolate its own handle.

skvelymake commented 11 hours ago

Debugging this I think I have found a separate issue. Technically if you only create Isolate instances with some memory allocated in them, the GC in the main isolate never runs.

RUN AS node --trace-gc --expose-gc script.js

let ivm = require('isolated-vm');

async function run() {
    setInterval(() => {
        const memoryUsage = process.memoryUsage();
        const rssMemory = Math.round(memoryUsage.rss / 1024 / 1024);

        console.log(`RSS memory: ${rssMemory} MB`);
    }, 1000);

    while (true) {
        {
            let isolate = new ivm.Isolate;
            let context = isolate.createContextSync();
            const allocatedSize = 10 * 1024 * 1024;
            context.evalSync(`
            const data = new ArrayBuffer(${allocatedSize});
            globalThis.data = data;
            const view = new Uint8Array(data);
            view.fill(123);
            void 0;
            `);
        }

        await new Promise(resolve => setTimeout(resolve, 10));
    }
}

run().catch(console.error);
[509708:0x61f0119cb3d0]     1332 ms: Scavenge 4.9 (5.0) -> 4.2 (6.0) MB, 49.87 / 49.48 ms  (average mu = 1.000, current mu = 1.000) task; 
RSS memory: 103 MB
RSS memory: 837 MB
[509708:0x61f0119cb3d0]     3200 ms: Scavenge 4.9 (6.2) -> 4.5 (5.7) MB, 14.48 / 14.10 ms  (average mu = 1.000, current mu = 1.000) task; 
RSS memory: 1395 MB
RSS memory: 1449 MB
RSS memory: 1951 MB
RSS memory: 2701 MB
[509708:0x61f0119cb3d0]     7096 ms: Scavenge 5.4 (5.7) -> 4.5 (5.7) MB, 31.39 / 31.13 ms  (average mu = 1.000, current mu = 1.000) allocation failure; 
RSS memory: 3075 MB
RSS memory: 3129 MB
RSS memory: 3182 MB
RSS memory: 3236 MB
[509708:0x61f0119cb3d0]    10645 ms: Scavenge 5.4 (5.7) -> 4.5 (5.7) MB, 31.82 / 31.64 ms  (average mu = 1.000, current mu = 1.000) allocation failure; 
RSS memory: 3215 MB
RSS memory: 3268 MB
RSS memory: 3322 MB
RSS memory: 3508 MB
[509708:0x61f0119cb3d0]    14968 ms: Scavenge 5.4 (5.7) -> 4.5 (5.7) MB, 37.50 / 37.32 ms  (average mu = 1.000, current mu = 1.000) allocation failure; 
RSS memory: 3760 MB
RSS memory: 3814 MB
RSS memory: 3869 MB
^C

If you run it, it only ever does Scavenge it never does full GC aka Mark-Compact. Therefore the Isolate memory is never freed.

I think the problem might be in how IVM reports allocated memory. Because I can see only RSS grow, but not external memory, which if I understand it correctly should grow as well.

laverdet commented 6 hours ago

The two garbage collectors run independently of one and other. You need to dispose of isolates when you are done with them otherwise you will run into memory issues.