DoneJS/quickjs-done-nextgen
1
0
Fork 0
mirror of https://github.com/DoneJS-Runtime/quickjs-done-nextgen.git synced 2025-01-09 17:43:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fix thread-safety issue in quickjs-libc (#578)

Browse source 
`JS_NewClassID(rt, &class_id)` where `class_id` is a global variable
is unsafe when called from multiple threads but that is exactly what
quickjs-libc.c did.

Add a new JS_AddRuntimeFinalizer function that lets quickjs-libc
store the class ids in JSRuntimeState and defer freeing the memory
until the runtime is destroyed. Necessary because object finalizers
such as js_std_file_finalizer need to know the class id and run after
js_std_free_handlers runs.

Fixes: https://github.com/quickjs-ng/quickjs/issues/577
This commit is contained in:
Ben Noordhuis 2024-10-07 21:27:38 +02:00 committed by GitHub
parent 27715a46bb
commit 9a37c57779
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 69 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
quickjs-libc.c
View file

@ -164,6 +164,8 @@ typedef struct JSThreadState {
JSValue exc; /* current exception from one of our handlers */ JSValue exc; /* current exception from one of our handlers */
/* not used in the main thread */ /* not used in the main thread */
JSWorkerMessagePipe *recv_pipe, *send_pipe; JSWorkerMessagePipe *recv_pipe, *send_pipe;
JSClassID std_file_class_id;
JSClassID worker_class_id;
} JSThreadState; } JSThreadState;
static uint64_t os_pending_signals; static uint64_t os_pending_signals;
@ -874,8 +876,6 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val,
return ret; return ret;
} }
static JSClassID js_std_file_class_id;
typedef struct { typedef struct {
FILE *f; FILE *f;
BOOL is_popen; BOOL is_popen;
@ -888,7 +888,8 @@ static BOOL is_stdio(FILE *f)
static void js_std_file_finalizer(JSRuntime *rt, JSValue val) static void js_std_file_finalizer(JSRuntime *rt, JSValue val)
{ {
JSSTDFile *s = JS_GetOpaque(val, js_std_file_class_id); JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSSTDFile *s = JS_GetOpaque(val, ts->std_file_class_id);
if (s) { if (s) {
if (s->f && !is_stdio(s->f)) { if (s->f && !is_stdio(s->f)) {
#if !defined(__wasi__) #if !defined(__wasi__)
@ -920,9 +921,11 @@ static JSValue js_std_strerror(JSContext *ctx, JSValue this_val,
static JSValue js_new_std_file(JSContext *ctx, FILE *f, BOOL is_popen) static JSValue js_new_std_file(JSContext *ctx, FILE *f, BOOL is_popen)
{ {
JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSSTDFile *s; JSSTDFile *s;
JSValue obj; JSValue obj;
obj = JS_NewObjectClass(ctx, js_std_file_class_id); obj = JS_NewObjectClass(ctx, ts->std_file_class_id);
if (JS_IsException(obj)) if (JS_IsException(obj))
return obj; return obj;
s = js_mallocz(ctx, sizeof(*s)); s = js_mallocz(ctx, sizeof(*s));
@ -1078,7 +1081,9 @@ static JSValue js_std_printf(JSContext *ctx, JSValue this_val,
static FILE *js_std_file_get(JSContext *ctx, JSValue obj) static FILE *js_std_file_get(JSContext *ctx, JSValue obj)
{ {
JSSTDFile *s = JS_GetOpaque2(ctx, obj, js_std_file_class_id); JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSSTDFile *s = JS_GetOpaque2(ctx, obj, ts->std_file_class_id);
if (!s) if (!s)
return NULL; return NULL;
if (!s->f) { if (!s->f) {
@ -1117,7 +1122,9 @@ static JSValue js_std_file_puts(JSContext *ctx, JSValue this_val,
static JSValue js_std_file_close(JSContext *ctx, JSValue this_val, static JSValue js_std_file_close(JSContext *ctx, JSValue this_val,
int argc, JSValue *argv) int argc, JSValue *argv)
{ {
JSSTDFile *s = JS_GetOpaque2(ctx, this_val, js_std_file_class_id); JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSSTDFile *s = JS_GetOpaque2(ctx, this_val, ts->std_file_class_id);
int err; int err;
if (!s) if (!s)
return JS_EXCEPTION; return JS_EXCEPTION;
@ -1633,16 +1640,17 @@ static int js_std_init(JSContext *ctx, JSModuleDef *m)
{ {
JSValue proto; JSValue proto;
JSRuntime *rt = JS_GetRuntime(ctx); JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
/* FILE class */ /* FILE class */
/* the class ID is created once */ /* the class ID is created once */
JS_NewClassID(rt, &js_std_file_class_id); JS_NewClassID(rt, &ts->std_file_class_id);
/* the class is created once per runtime */ /* the class is created once per runtime */
JS_NewClass(rt, js_std_file_class_id, &js_std_file_class); JS_NewClass(rt, ts->std_file_class_id, &js_std_file_class);
proto = JS_NewObject(ctx); proto = JS_NewObject(ctx);
JS_SetPropertyFunctionList(ctx, proto, js_std_file_proto_funcs, JS_SetPropertyFunctionList(ctx, proto, js_std_file_proto_funcs,
countof(js_std_file_proto_funcs)); countof(js_std_file_proto_funcs));
JS_SetClassProto(ctx, js_std_file_class_id, proto); JS_SetClassProto(ctx, ts->std_file_class_id, proto);
JS_SetModuleExportList(ctx, m, js_std_funcs, JS_SetModuleExportList(ctx, m, js_std_funcs,
countof(js_std_funcs)); countof(js_std_funcs));
@ -3278,7 +3286,6 @@ typedef struct {
uint64_t buf[]; uint64_t buf[];
} JSSABHeader; } JSSABHeader;
static JSClassID js_worker_class_id;
static JSContext *(*js_worker_new_context_func)(JSRuntime *rt); static JSContext *(*js_worker_new_context_func)(JSRuntime *rt);
static int atomic_add_int(int *ptr, int v) static int atomic_add_int(int *ptr, int v)
@ -3391,7 +3398,8 @@ static void js_free_port(JSRuntime *rt, JSWorkerMessageHandler *port)
static void js_worker_finalizer(JSRuntime *rt, JSValue val) static void js_worker_finalizer(JSRuntime *rt, JSValue val)
{ {
JSWorkerData *worker = JS_GetOpaque(val, js_worker_class_id); JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSWorkerData *worker = JS_GetOpaque(val, ts->worker_class_id);
if (worker) { if (worker) {
js_free_message_pipe(worker->recv_pipe); js_free_message_pipe(worker->recv_pipe);
js_free_message_pipe(worker->send_pipe); js_free_message_pipe(worker->send_pipe);
@ -3459,18 +3467,20 @@ static JSValue js_worker_ctor_internal(JSContext *ctx, JSValue new_target,
JSWorkerMessagePipe *recv_pipe, JSWorkerMessagePipe *recv_pipe,
JSWorkerMessagePipe *send_pipe) JSWorkerMessagePipe *send_pipe)
{ {
JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSValue obj = JS_UNDEFINED, proto; JSValue obj = JS_UNDEFINED, proto;
JSWorkerData *s; JSWorkerData *s;
/* create the object */ /* create the object */
if (JS_IsUndefined(new_target)) { if (JS_IsUndefined(new_target)) {
proto = JS_GetClassProto(ctx, js_worker_class_id); proto = JS_GetClassProto(ctx, ts->worker_class_id);
} else { } else {
proto = JS_GetPropertyStr(ctx, new_target, "prototype"); proto = JS_GetPropertyStr(ctx, new_target, "prototype");
if (JS_IsException(proto)) if (JS_IsException(proto))
goto fail; goto fail;
} }
obj = JS_NewObjectProtoClass(ctx, proto, js_worker_class_id); obj = JS_NewObjectProtoClass(ctx, proto, ts->worker_class_id);
JS_FreeValue(ctx, proto); JS_FreeValue(ctx, proto);
if (JS_IsException(obj)) if (JS_IsException(obj))
goto fail; goto fail;
@ -3574,7 +3584,9 @@ static JSValue js_worker_ctor(JSContext *ctx, JSValue new_target,
static JSValue js_worker_postMessage(JSContext *ctx, JSValue this_val, static JSValue js_worker_postMessage(JSContext *ctx, JSValue this_val,
int argc, JSValue *argv) int argc, JSValue *argv)
{ {
JSWorkerData *worker = JS_GetOpaque2(ctx, this_val, js_worker_class_id); JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSWorkerData *worker = JS_GetOpaque2(ctx, this_val, ts->worker_class_id);
JSWorkerMessagePipe *ps; JSWorkerMessagePipe *ps;
size_t data_len, i; size_t data_len, i;
uint8_t *data; uint8_t *data;
@ -3653,7 +3665,7 @@ static JSValue js_worker_set_onmessage(JSContext *ctx, JSValue this_val,
{ {
JSRuntime *rt = JS_GetRuntime(ctx); JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt); JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSWorkerData *worker = JS_GetOpaque2(ctx, this_val, js_worker_class_id); JSWorkerData *worker = JS_GetOpaque2(ctx, this_val, ts->worker_class_id);
JSWorkerMessageHandler *port; JSWorkerMessageHandler *port;
if (!worker) if (!worker)
@ -3685,7 +3697,9 @@ static JSValue js_worker_set_onmessage(JSContext *ctx, JSValue this_val,
static JSValue js_worker_get_onmessage(JSContext *ctx, JSValue this_val) static JSValue js_worker_get_onmessage(JSContext *ctx, JSValue this_val)
{ {
JSWorkerData *worker = JS_GetOpaque2(ctx, this_val, js_worker_class_id); JSRuntime *rt = JS_GetRuntime(ctx);
JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSWorkerData *worker = JS_GetOpaque2(ctx, this_val, ts->worker_class_id);
JSWorkerMessageHandler *port; JSWorkerMessageHandler *port;
if (!worker) if (!worker)
return JS_EXCEPTION; return JS_EXCEPTION;
@ -3838,8 +3852,8 @@ static int js_os_init(JSContext *ctx, JSModuleDef *m)
JSThreadState *ts = JS_GetRuntimeOpaque(rt); JSThreadState *ts = JS_GetRuntimeOpaque(rt);
JSValue proto, obj; JSValue proto, obj;
/* Worker class */ /* Worker class */
JS_NewClassID(rt, &js_worker_class_id); JS_NewClassID(rt, &ts->worker_class_id);
JS_NewClass(rt, js_worker_class_id, &js_worker_class); JS_NewClass(rt, ts->worker_class_id, &js_worker_class);
proto = JS_NewObject(ctx); proto = JS_NewObject(ctx);
JS_SetPropertyFunctionList(ctx, proto, js_worker_proto_funcs, countof(js_worker_proto_funcs)); JS_SetPropertyFunctionList(ctx, proto, js_worker_proto_funcs, countof(js_worker_proto_funcs));
@ -3847,7 +3861,7 @@ static int js_os_init(JSContext *ctx, JSModuleDef *m)
JS_CFUNC_constructor, 0); JS_CFUNC_constructor, 0);
JS_SetConstructor(ctx, obj, proto); JS_SetConstructor(ctx, obj, proto);
JS_SetClassProto(ctx, js_worker_class_id, proto); JS_SetClassProto(ctx, ts->worker_class_id, proto);
/* set 'Worker.parent' if necessary */ /* set 'Worker.parent' if necessary */
if (ts->recv_pipe && ts->send_pipe) { if (ts->recv_pipe && ts->send_pipe) {
@ -3961,7 +3975,7 @@ void js_std_init_handlers(JSRuntime *rt)
{ {
JSThreadState *ts; JSThreadState *ts;
ts = malloc(sizeof(*ts)); ts = js_malloc_rt(rt, sizeof(*ts));
if (!ts) { if (!ts) {
fprintf(stderr, "Could not allocate memory for the worker"); fprintf(stderr, "Could not allocate memory for the worker");
exit(1); exit(1);
@ -3976,6 +3990,7 @@ void js_std_init_handlers(JSRuntime *rt)
ts->exc = JS_UNDEFINED; ts->exc = JS_UNDEFINED;
JS_SetRuntimeOpaque(rt, ts); JS_SetRuntimeOpaque(rt, ts);
JS_AddRuntimeFinalizer(rt, js_free_rt, ts);
#ifdef USE_WORKER #ifdef USE_WORKER
/* set the SharedArrayBuffer memory handlers */ /* set the SharedArrayBuffer memory handlers */
@ -4015,9 +4030,6 @@ void js_std_free_handlers(JSRuntime *rt)
js_free_message_pipe(ts->recv_pipe); js_free_message_pipe(ts->recv_pipe);
js_free_message_pipe(ts->send_pipe); js_free_message_pipe(ts->send_pipe);
#endif #endif
free(ts);
JS_SetRuntimeOpaque(rt, NULL); /* fail safe */
} }
static void js_dump_obj(JSContext *ctx, FILE *f, JSValue val) static void js_dump_obj(JSContext *ctx, FILE *f, JSValue val)

27
quickjs.c
View file

@ -221,6 +221,12 @@ typedef struct JSMallocState {
void *opaque; /* user opaque */ void *opaque; /* user opaque */
} JSMallocState; } JSMallocState;
typedef struct JSRuntimeFinalizerState {
struct JSRuntimeFinalizerState *next;
JSRuntimeFinalizer *finalizer;
void *arg;
} JSRuntimeFinalizerState;
struct JSRuntime { struct JSRuntime {
JSMallocFunctions mf; JSMallocFunctions mf;
JSMallocState malloc_state; JSMallocState malloc_state;
@ -292,6 +298,7 @@ struct JSRuntime {
JSShape **shape_hash; JSShape **shape_hash;
bf_context_t bf_ctx; bf_context_t bf_ctx;
void *user_opaque; void *user_opaque;
JSRuntimeFinalizerState *finalizers;
}; };
struct JSClass { struct JSClass {
@ -1816,6 +1823,19 @@ void JS_SetRuntimeOpaque(JSRuntime *rt, void *opaque)
rt->user_opaque = opaque; rt->user_opaque = opaque;
} }
int JS_AddRuntimeFinalizer(JSRuntime *rt, JSRuntimeFinalizer *finalizer,
void *arg)
{
JSRuntimeFinalizerState *fs = js_malloc_rt(rt, sizeof(*fs));
if (!fs)
return -1;
fs->next = rt->finalizers;
fs->finalizer = finalizer;
fs->arg = arg;
rt->finalizers = fs;
return 0;
}
static void *js_def_calloc(void *opaque, size_t count, size_t size) static void *js_def_calloc(void *opaque, size_t count, size_t size)
{ {
return calloc(count, size); return calloc(count, size);
@ -2196,6 +2216,13 @@ void JS_FreeRuntime(JSRuntime *rt)
} }
#endif #endif
while (rt->finalizers) {
JSRuntimeFinalizerState *fs = rt->finalizers;
rt->finalizers = fs->next;
fs->finalizer(rt, fs->arg);
js_free_rt(rt, fs);
}
{ {
JSMallocState *ms = &rt->malloc_state; JSMallocState *ms = &rt->malloc_state;
rt->mf.js_free(ms->opaque, rt); rt->mf.js_free(ms->opaque, rt);

7
quickjs.h
View file

@ -287,6 +287,11 @@ typedef struct JSMallocFunctions {
size_t (*js_malloc_usable_size)(const void *ptr); size_t (*js_malloc_usable_size)(const void *ptr);
} JSMallocFunctions; } JSMallocFunctions;
// Finalizers run in LIFO order at the very end of JS_FreeRuntime.
// Intended for cleanup of associated resources; the runtime itself
// is no longer usable.
typedef void JSRuntimeFinalizer(JSRuntime *rt, void *arg);
typedef struct JSGCObjectHeader JSGCObjectHeader; typedef struct JSGCObjectHeader JSGCObjectHeader;
JS_EXTERN JSRuntime *JS_NewRuntime(void); JS_EXTERN JSRuntime *JS_NewRuntime(void);
@ -306,6 +311,8 @@ JS_EXTERN JSRuntime *JS_NewRuntime2(const JSMallocFunctions *mf, void *opaque);
JS_EXTERN void JS_FreeRuntime(JSRuntime *rt); JS_EXTERN void JS_FreeRuntime(JSRuntime *rt);
JS_EXTERN void *JS_GetRuntimeOpaque(JSRuntime *rt); JS_EXTERN void *JS_GetRuntimeOpaque(JSRuntime *rt);
JS_EXTERN void JS_SetRuntimeOpaque(JSRuntime *rt, void *opaque); JS_EXTERN void JS_SetRuntimeOpaque(JSRuntime *rt, void *opaque);
JS_EXTERN int JS_AddRuntimeFinalizer(JSRuntime *rt,
JSRuntimeFinalizer *finalizer, void *arg);
typedef void JS_MarkFunc(JSRuntime *rt, JSGCObjectHeader *gp); typedef void JS_MarkFunc(JSRuntime *rt, JSGCObjectHeader *gp);
JS_EXTERN void JS_MarkValue(JSRuntime *rt, JSValue val, JS_MarkFunc *mark_func); JS_EXTERN void JS_MarkValue(JSRuntime *rt, JSValue val, JS_MarkFunc *mark_func);
JS_EXTERN void JS_RunGC(JSRuntime *rt); JS_EXTERN void JS_RunGC(JSRuntime *rt);