quickjs icon indicating copy to clipboard operation
quickjs copied to clipboard

Can quickjs be used in multithreading?

Open penneryu opened this issue 1 year ago • 5 comments

static JSClassID js_class_id_alloc = JS_CLASS_INIT_COUNT;

/* a new class ID is allocated if *pclass_id != 0 */
JSClassID JS_NewClassID(JSClassID *pclass_id)
{
    JSClassID class_id;
#ifdef CONFIG_ATOMICS
    pthread_mutex_lock(&js_class_id_mutex);
#endif
    class_id = *pclass_id;
    if (class_id == 0) {
        class_id = js_class_id_alloc++;
        *pclass_id = class_id;
    }
#ifdef CONFIG_ATOMICS
    pthread_mutex_unlock(&js_class_id_mutex);
#endif
    return class_id;
}

As shown in the code above, js_class_id_alloc is a global variable, and the same reference is used in all threads. If different JSRuntimes are instantiated in different threads, and JS_NewClassID is called in each thread to generate a new classId, then the classId will not grow independently in each thread as expected, which should cause management problems for class_array in each JSRuntime.

ps: In addition, I found that calling quickjs API in multiple threads will cause some resource management problems, even in different JSRuntime instances.

penneryu avatar Oct 15 '24 10:10 penneryu

I don't know if Charlie and Fabrice intend to cherry-pick it but we fixed that in quickjs-ng almost a year ago to the day in commit quickjs-ng/quickjs@5ce2957e.

bnoordhuis avatar Nov 13 '24 15:11 bnoordhuis

I don't know if Charlie and Fabrice intend to cherry-pick it but we fixed that in quickjs-ng almost a year ago to the day in commit quickjs-ng/quickjs@5ce2957e.

thanks, I think this modification is good, classid is originally bound to the runtime, so creating a new one should not affect other runtimes.

penneryu avatar Nov 14 '24 02:11 penneryu

Yes, we shall cherry-pick this patch and many others

chqrlie avatar Nov 16 '24 08:11 chqrlie

Note that in normal cases you want to share the class ID between all the JSRuntimes so that they can easily communicate. In this case, locking is not needed and it explains the current code.

bellard avatar Mar 19 '25 12:03 bellard

Note that in normal cases you want to share the class ID between all the JSRuntimes so that they can easily communicate. In this case, locking is not needed and it explains the current code.

I understand, but in our business scenario (it is a mobile app), one of the threads of quickjs is used for the web environment (there are many window and other web dom objects), and another thread of quickjs will be used in a pure javascript environment similar to node. Their JSRuntime does not need to share any information. Maybe our case is a special case, but maybe not.

penneryu avatar May 06 '25 11:05 penneryu