Skip to content

Commit 9b58030

Browse files
bnoordhuisbnoordhuis
authored
Fix atom leak in JS_NewSymbol (#1461)
Add a new JS_ABORT_ON_LEAKS flag for use in api-test.c to help catch regressions.
1 parent c707cf5 commit 9b58030

3 files changed

Lines changed: 44 additions & 20 deletions

File tree

‎api-test.c‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@
77
#include "quickjs.h"
88
#include "cutils.h"
99

10+
static JSRuntime *new_runtime(void)
11+
{
12+
JSRuntime *rt = JS_NewRuntime();
13+
14+
if (rt)
15+
JS_SetDumpFlags(rt, JS_ABORT_ON_LEAKS);
16+
return rt;
17+
}
18+
1019
static JSValue eval(JSContext *ctx, const char *code)
1120
{
1221
return JS_Eval(ctx, code, strlen(code), "<input>", JS_EVAL_TYPE_GLOBAL);
@@ -46,7 +55,7 @@ static void cfunctions(void)
4655
const char *s;
4756
JSValue ret, stack;
4857

49-
JSRuntime *rt = JS_NewRuntime();
58+
JSRuntime *rt = new_runtime();
5059
JSContext *ctx = JS_NewContext(rt);
5160
JSValue cfunc = JS_NewCFunction(ctx, cfunc_callback, "cfunc", 42);
5261
JSValue cfuncdata =
@@ -179,7 +188,7 @@ static void sync_call(void)
179188
} catch (e) {} \
180189
})();";
181190

182-
JSRuntime *rt = JS_NewRuntime();
191+
JSRuntime *rt = new_runtime();
183192
JSContext *ctx = JS_NewContext(rt);
184193
int time = 0;
185194
JS_SetInterruptHandler(rt, timeout_interrupt_handler, &time);
@@ -206,7 +215,7 @@ static void async_call(void)
206215
await loop().catch(() => {}); \
207216
})();";
208217

209-
JSRuntime *rt = JS_NewRuntime();
218+
JSRuntime *rt = new_runtime();
210219
JSContext *ctx = JS_NewContext(rt);
211220
int time = 0;
212221
JS_SetInterruptHandler(rt, timeout_interrupt_handler, &time);
@@ -250,7 +259,7 @@ static void async_call_stack_overflow(void)
250259
} \
251260
})();";
252261

253-
JSRuntime *rt = JS_NewRuntime();
262+
JSRuntime *rt = new_runtime();
254263
JSContext *ctx = JS_NewContext(rt);
255264
JSValue value = JS_UNDEFINED;
256265
JS_SetContextOpaque(ctx, &value);
@@ -276,7 +285,7 @@ static void async_call_stack_overflow(void)
276285
// https://github.com/quickjs-ng/quickjs/issues/914
277286
static void raw_context_global_var(void)
278287
{
279-
JSRuntime *rt = JS_NewRuntime();
288+
JSRuntime *rt = new_runtime();
280289
JSContext *ctx = JS_NewContextRaw(rt);
281290
JS_AddIntrinsicEval(ctx);
282291
{
@@ -300,7 +309,7 @@ static void raw_context_global_var(void)
300309

301310
static void is_array(void)
302311
{
303-
JSRuntime *rt = JS_NewRuntime();
312+
JSRuntime *rt = new_runtime();
304313
JSContext *ctx = JS_NewContext(rt);
305314
{
306315
JSValue ret = eval(ctx, "[]");
@@ -347,7 +356,7 @@ static JSModuleDef *loader(JSContext *ctx, const char *name, void *opaque)
347356

348357
static void module_serde(void)
349358
{
350-
JSRuntime *rt = JS_NewRuntime();
359+
JSRuntime *rt = new_runtime();
351360
//JS_SetDumpFlags(rt, JS_DUMP_MODULE_RESOLVE);
352361
JS_SetModuleLoaderFunc(rt, NULL, loader, NULL);
353362
JSContext *ctx = JS_NewContext(rt);
@@ -384,7 +393,7 @@ static void module_serde(void)
384393

385394
static void runtime_cstring_free(void)
386395
{
387-
JSRuntime *rt = JS_NewRuntime();
396+
JSRuntime *rt = new_runtime();
388397
JSContext *ctx = JS_NewContext(rt);
389398
// string -> cstring + JS_FreeCStringRT
390399
{
@@ -422,7 +431,7 @@ static void runtime_cstring_free(void)
422431

423432
static void utf16_string(void)
424433
{
425-
JSRuntime *rt = JS_NewRuntime();
434+
JSRuntime *rt = new_runtime();
426435
JSContext *ctx = JS_NewContext(rt);
427436
{
428437
JSValue v = JS_NewStringUTF16(ctx, NULL, 0);
@@ -493,7 +502,7 @@ function addItem() { \
493502
}";
494503
static const char test_code[] = "addItem()";
495504

496-
JSRuntime *rt = JS_NewRuntime();
505+
JSRuntime *rt = new_runtime();
497506
JSContext *ctx = JS_NewContext(rt);
498507

499508
JSValue ret = eval(ctx, init_code);
@@ -549,7 +558,7 @@ static void promise_hook(void)
549558
{
550559
int *cc = promise_hook_state.hook_type_call_count;
551560
JSContext *unused;
552-
JSRuntime *rt = JS_NewRuntime();
561+
JSRuntime *rt = new_runtime();
553562
//JS_SetDumpFlags(rt, JS_DUMP_PROMISE);
554563
JS_SetPromiseHook(rt, promise_hook_cb, &promise_hook_state);
555564
JSContext *ctx = JS_NewContext(rt);
@@ -693,7 +702,7 @@ static void dump_memory_usage(void)
693702
JSRuntime *rt = NULL;
694703
JSContext *ctx = NULL;
695704

696-
rt = JS_NewRuntime();
705+
rt = new_runtime();
697706
ctx = JS_NewContext(rt);
698707

699708
//JS_SetDumpFlags(rt, JS_DUMP_PROMISE);
@@ -734,7 +743,7 @@ static void new_errors(void)
734743
};
735744
const Entry *e;
736745

737-
JSRuntime *rt = JS_NewRuntime();
746+
JSRuntime *rt = new_runtime();
738747
JSContext *ctx = JS_NewContext(rt);
739748
for (e = entries; e < endof(entries); e++) {
740749
JSValue obj = (*e->func)(ctx, "the %s", "needle");
@@ -788,7 +797,7 @@ static void global_object_prototype(void)
788797
int res;
789798

790799
{
791-
rt = JS_NewRuntime();
800+
rt = new_runtime();
792801
ctx = JS_NewContext(rt);
793802
proto = JS_NewObject(ctx);
794803
assert(JS_IsObject(proto));
@@ -817,7 +826,7 @@ static void global_object_prototype(void)
817826
.class_name = "Global Object",
818827
.exotic = &exotic,
819828
};
820-
rt = JS_NewRuntime();
829+
rt = new_runtime();
821830
class_id = 0;
822831
JS_NewClassID(rt, &class_id);
823832
res = JS_NewClass(rt, class_id, &def);
@@ -845,7 +854,7 @@ static void global_object_prototype(void)
845854
// https://github.com/quickjs-ng/quickjs/issues/1178
846855
static void slice_string_tocstring(void)
847856
{
848-
JSRuntime *rt = JS_NewRuntime();
857+
JSRuntime *rt = new_runtime();
849858
JSContext *ctx = JS_NewContext(rt);
850859
JSValue ret = eval(ctx, "'.'.repeat(16384).slice(1, -1)");
851860
assert(!JS_IsException(ret));
@@ -865,7 +874,7 @@ static void immutable_array_buffer(void)
865874
char buf[96];
866875
int i, v;
867876

868-
JSRuntime *rt = JS_NewRuntime();
877+
JSRuntime *rt = new_runtime();
869878
JSContext *ctx = JS_NewContext(rt);
870879
for (i = 0; i < 2; i++) {
871880
obj = JS_NewObject(ctx);
@@ -907,7 +916,7 @@ static void immutable_array_buffer(void)
907916

908917
static void get_uint8array(void)
909918
{
910-
JSRuntime *rt = JS_NewRuntime();
919+
JSRuntime *rt = new_runtime();
911920
JSContext *ctx = JS_NewContext(rt);
912921
JSValue val;
913922
uint8_t *p;
@@ -964,7 +973,7 @@ static void get_uint8array(void)
964973

965974
static void new_symbol(void)
966975
{
967-
JSRuntime *rt = JS_NewRuntime();
976+
JSRuntime *rt = new_runtime();
968977
JSContext *ctx = JS_NewContext(rt);
969978
JSValue global = JS_GetGlobalObject(ctx);
970979
JSValue sym, ret;

‎quickjs.c‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2265,6 +2265,7 @@ void JS_SetRuntimeInfo(JSRuntime *rt, const char *s)
22652265
void JS_FreeRuntime(JSRuntime *rt)
22662266
{
22672267
struct list_head *el, *el1;
2268+
bool leak = false;
22682269
int i;
22692270

22702271
rt->in_free = true;
@@ -2305,6 +2306,7 @@ void JS_FreeRuntime(JSRuntime *rt)
23052306
header_done = true;
23062307
}
23072308
JS_DumpGCObject(rt, p);
2309+
leak = true;
23082310
}
23092311
}
23102312

@@ -2381,6 +2383,7 @@ void JS_FreeRuntime(JSRuntime *rt)
23812383
} else {
23822384
printf("\n");
23832385
}
2386+
leak = true;
23842387
}
23852388
}
23862389
}
@@ -2429,6 +2432,7 @@ void JS_FreeRuntime(JSRuntime *rt)
24292432
}
24302433
if (rt->rt_info)
24312434
printf("\n");
2435+
leak = true;
24322436
}
24332437
#endif
24342438

@@ -2448,14 +2452,20 @@ void JS_FreeRuntime(JSRuntime *rt)
24482452
printf("Memory leak: %zd bytes lost in %zd block%s\n",
24492453
s->malloc_size - sizeof(JSRuntime),
24502454
s->malloc_count - 1, &"s"[s->malloc_count == 2]);
2455+
leak = true;
24512456
}
24522457
}
24532458
#endif
24542459

2460+
leak &= check_dump_flag(rt, JS_ABORT_ON_LEAKS);
2461+
24552462
{
24562463
JSMallocState *ms = &rt->malloc_state;
24572464
rt->mf.js_free(ms->opaque, rt);
24582465
}
2466+
2467+
if (leak)
2468+
abort();
24592469
}
24602470

24612471
JSContext *JS_NewContextRaw(JSRuntime *rt)
@@ -3415,7 +3425,11 @@ JSValue JS_NewSymbol(JSContext *ctx, const char *description, bool is_global)
34153425
JSAtom atom = JS_NewAtom(ctx, description);
34163426
if (atom == JS_ATOM_NULL)
34173427
return JS_EXCEPTION;
3418-
return JS_NewSymbolFromAtom(ctx, atom, is_global ? JS_ATOM_TYPE_GLOBAL_SYMBOL : JS_ATOM_TYPE_SYMBOL);
3428+
int atom_type =
3429+
is_global ? JS_ATOM_TYPE_GLOBAL_SYMBOL : JS_ATOM_TYPE_SYMBOL;
3430+
JSValue symbol = JS_NewSymbolFromAtom(ctx, atom, atom_type);
3431+
JS_FreeAtom(ctx, atom);
3432+
return symbol;
34193433
}
34203434

34213435
#define ATOM_GET_STR_BUF_SIZE 64

‎quickjs.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,7 @@ typedef struct JSMallocFunctions {
499499
#define JS_DUMP_OBJECTS 0x20000 /* dump objects in JS_FreeRuntime */
500500
#define JS_DUMP_ATOMS 0x40000 /* dump atoms in JS_FreeRuntime */
501501
#define JS_DUMP_SHAPES 0x80000 /* dump shapes in JS_FreeRuntime */
502+
#define JS_ABORT_ON_LEAKS 0x10C000 /* abort on atom/object/string leaks; for testing */
502503

503504
// Finalizers run in LIFO order at the very end of JS_FreeRuntime.
504505
// Intended for cleanup of associated resources; the runtime itself

0 commit comments

Comments
 (0)