Skip to content

Commit

Permalink
timers: fix leak when interpreter shutsdown early
Browse files Browse the repository at this point in the history
If something creates an unhandled rejection condition we'll end up with
timers being kept alive in C without a way to recover them.

Fix it by keeping the strong reference in JS, so it all goes away when
we destroy the runtime.
  • Loading branch information
saghul committed Nov 7, 2023
1 parent dfda088 commit 5ce349a
Show file tree
Hide file tree
Showing 8 changed files with 15,088 additions and 15,009 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ src/js/std.js
src/version.h
node_modules/
docs/api/
test_dir*/
30,021 changes: 15,027 additions & 14,994 deletions src/bundles/c/core/polyfills.c

Large diffs are not rendered by default.

5 changes: 1 addition & 4 deletions src/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ int main(int argc, char **argv) {

int exit_code = TJS_Run(qrt);

if (exit_code == EXIT_SUCCESS) {
// TODO: maybe mark the runtime as aborted and skip some steps?
TJS_FreeRuntime(qrt);
}
TJS_FreeRuntime(qrt);

return exit_code;
}
6 changes: 1 addition & 5 deletions src/js/polyfills/base.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
const core = globalThis.__bootstrap;
import './timers.js';

import queueMicrotask from 'queue-microtask';

globalThis.setTimeout = core.setTimeout;
globalThis.clearTimeout = core.clearTimeout;
globalThis.setInterval = core.setInterval;
globalThis.clearInterval = core.clearInterval;
globalThis.queueMicrotask = queueMicrotask;

Object.defineProperty(globalThis, 'global', {
Expand Down
29 changes: 29 additions & 0 deletions src/js/polyfills/timers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const core = globalThis.__bootstrap;

const timers = new Set();

globalThis.setTimeout = (fn, ms) => {
const t = core.setTimeout(fn, ms);

timers.add(t);

return t;
};

globalThis.clearTimeout = t => {
core.clearTimeout(t);
timers.delete(t);
};

globalThis.setInterval = (fn, ms) => {
const t = core.setInterval(fn, ms);

timers.add(t);

return t;
};

globalThis.clearInterval = t => {
core.clearInterval(t);
timers.delete(t);
};
6 changes: 0 additions & 6 deletions src/timers.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ typedef struct {
JSContext *ctx;
uv_timer_t handle;
int interval;
JSValue obj;
JSValue func;
int argc;
JSValue argv[];
Expand All @@ -47,10 +46,6 @@ static void clear_timer(TJSTimer *th) {
th->argv[i] = JS_UNDEFINED;
}
th->argc = 0;

JSValue obj = th->obj;
th->obj = JS_UNDEFINED;
JS_FreeValue(ctx, obj);
}

static void call_timer(TJSTimer *th) {
Expand Down Expand Up @@ -145,7 +140,6 @@ static JSValue tjs_setTimeout(JSContext *ctx, JSValueConst this_val, int argc, J
CHECK_EQ(uv_timer_init(tjs_get_loop(ctx), &th->handle), 0);
th->handle.data = th;
th->interval = magic;
th->obj = JS_DupValue(ctx, obj);
th->func = JS_DupValue(ctx, func);
th->argc = nargs;
for (int i = 0; i < nargs; i++)
Expand Down
13 changes: 13 additions & 0 deletions tests/helpers/timers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
async function foo() {
throw new Error('oops!');
}


setTimeout(async () => {
await foo();
}, 100);


setTimeout(() => {
console.log('boooo!');
}, 99999);
16 changes: 16 additions & 0 deletions tests/test-timer-leak.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import assert from 'tjs:assert';
import path from 'tjs:path';


const args = [
tjs.exepath,
'run',
path.join(import.meta.dirname, 'helpers', 'timers.js')
];
const proc = tjs.spawn(args, { stdout: 'ignore', stderr: 'pipe' });
const buf = new Uint8Array(4096);
const nread = await proc.stderr.read(buf);
const stderrStr = new TextDecoder().decode(buf.subarray(0, nread));
const status = await proc.wait();
assert.ok(stderrStr.match(/Error: oops!/) !== null, 'dumps to stderr');
assert.ok(status.exit_status !== 0 && status.term_signal === null, 'succeeded');

0 comments on commit 5ce349a

Please sign in to comment.