Fix thread-safety issue in quickjs-libc

`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.

Change the class ids in quickjs-libc.c to thread-locals, and change
the function prototype of JS_NewClassID to hopefully make it harder
for downstream users to introduce such bugs.

The gcc48 and tcc buildbots don't support _Thread_local and that is
why this commit removes them.

I toyed with the idea of porting the uv_key_create/uv_key_get/etc.
functions from libuv but because quickjs-libc doesn't know when it's
safe to free the thread-local keys, that leads to memory leaks, and
support for ancient or niche compilers isn't worth that.

Fixes: #577

Author: bnoordhuis

.github/workflows/ci.yml

@@ -55,7 +55,6 @@ jobs:
           - { os: ubuntu-latest, configType: asan, runTest262: true }
           - { os: ubuntu-latest, configType: ubsan, runTest262: true }
           - { os: ubuntu-latest, configType: msan }
-          - { os: ubuntu-latest, configType: tcc }
           - { os: ubuntu-latest, arch: x86 }
           - { os: ubuntu-latest, arch: riscv64 }
           - { os: ubuntu-latest, arch: s390x }
@@ -84,20 +83,6 @@ jobs:
         run: |
           sudo sysctl -w kernel.randomize_va_space=0
 
-      - name: install TCC
-        if: ${{ matrix.config.configType == 'tcc' }}
-        run: |
-          pushd /tmp
-          git clone https://repo.or.cz/tinycc.git
-          cd tinycc
-          git checkout 9d2068c6309dc50dfdbbc30a5d6757683d3f884c
-          ./configure
-          make
-          sudo make install
-          tcc -v
-          popd
-          echo "CC=tcc" >> $GITHUB_ENV;
-
       - name: Setup Alpine
         if: ${{ matrix.config.arch != '' }}
         uses: jirutka/setup-alpine@v1
@@ -160,38 +145,6 @@ jobs:
         run: |
           time make test262
 
-  linux-gcc48:
-    runs-on: ubuntu-latest
-    container:
-      image: ubuntu:18.04
-    # Work around an issue where the node from actions/checkout is too new
-    # to run inside the old container image.
-    env:
-      ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
-    steps:
-      - name: install dependencies
-        run: |
-          apt update && apt -y install make gcc-4.8 cmake software-properties-common
-          # git in default ppa repository is too old to run submodule checkout
-          add-apt-repository -y ppa:git-core/ppa
-          apt update && apt install -y git
-      - name: checkout
-        uses: actions/checkout@v3
-        with:
-          submodules: true
-      - name: build
-        env:
-          CC: gcc-4.8
-        run: |
-          mkdir build
-          cd build
-          cmake ..
-          cd ..
-          make -C build -j$(getconf _NPROCESSORS_ONLN)
-      - name: stats
-        run: |
-          ./build/qjs -qd
-
   windows-msvc:
     runs-on: windows-latest
     strategy:

CMakeLists.txt

@@ -282,15 +282,7 @@ endif()
 # Test262 runner
 #
 
-if(WIN32
-OR EMSCRIPTEN
-OR CMAKE_C_COMPILER_ID STREQUAL "TinyCC"
-OR CMAKE_C_COMPILER_ID STREQUAL "GNU" AND CMAKE_C_COMPILER_VERSION VERSION_LESS 5)
-    # Empty. run-test262 uses pthreads, sorry Windows users.
-    # tcc and gcc 4.8 don't understand _Thread_local, whereas I
-    # don't understand why people still use 4.8 in this day and age
-    # but hey, here we are.
-else()
+if(NOT WIN32)
     add_executable(run-test262
         run-test262.c
     )

examples/point.c

@@ -126,7 +126,7 @@ static int js_point_init(JSContext *ctx, JSModuleDef *m)
     JSRuntime *rt = JS_GetRuntime(ctx);
 
     /* create the Point class */
-    JS_NewClassID(rt, &js_point_class_id);
+    js_point_class_id = JS_NewClassID(rt);
     JS_NewClass(rt, js_point_class_id, &js_point_class);
 
     point_proto = JS_NewObject(ctx);

quickjs-libc.c

@@ -874,7 +874,7 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val,
     return ret;
 }
 
-static JSClassID js_std_file_class_id;
+static _Thread_local JSClassID js_std_file_class_id;
 
 typedef struct {
     FILE *f;
@@ -1633,7 +1633,7 @@ static int js_std_init(JSContext *ctx, JSModuleDef *m)
 
     /* FILE class */
     /* the class ID is created once */
-    JS_NewClassID(rt, &js_std_file_class_id);
+    js_std_file_class_id = JS_NewClassID(rt);
     /* the class is created once per runtime */
     JS_NewClass(rt, js_std_file_class_id, &js_std_file_class);
     proto = JS_NewObject(ctx);
@@ -3275,7 +3275,7 @@ typedef struct {
     uint64_t buf[];
 } JSSABHeader;
 
-static JSClassID js_worker_class_id;
+static _Thread_local JSClassID js_worker_class_id;
 static JSContext *(*js_worker_new_context_func)(JSRuntime *rt);
 
 static int atomic_add_int(int *ptr, int v)
@@ -3835,7 +3835,7 @@ static int js_os_init(JSContext *ctx, JSModuleDef *m)
         JSThreadState *ts = JS_GetRuntimeOpaque(rt);
         JSValue proto, obj;
         /* Worker class */
-        JS_NewClassID(rt, &js_worker_class_id);
+        js_worker_class_id = JS_NewClassID(rt);
         JS_NewClass(rt, js_worker_class_id, &js_worker_class);
         proto = JS_NewObject(ctx);
         JS_SetPropertyFunctionList(ctx, proto, js_worker_proto_funcs, countof(js_worker_proto_funcs));

quickjs.c

@@ -3441,15 +3441,9 @@ static inline BOOL JS_IsEmptyString(JSValue v)
 
 /* JSClass support */
 
-/* a new class ID is allocated if *pclass_id != 0 */
-JSClassID JS_NewClassID(JSRuntime *rt, JSClassID *pclass_id)
+JSClassID JS_NewClassID(JSRuntime *rt)
 {
-    JSClassID class_id = *pclass_id;
-    if (class_id == 0) {
-        class_id = rt->js_class_id_alloc++;
-        *pclass_id = class_id;
-    }
-    return class_id;
+    return rt->js_class_id_alloc++;
 }
 
 JSClassID JS_GetClassID(JSValue v)

quickjs.h

@@ -466,7 +466,7 @@ typedef struct JSClassDef {
 } JSClassDef;
 
 #define JS_INVALID_CLASS_ID 0
-JS_EXTERN JSClassID JS_NewClassID(JSRuntime *rt, JSClassID *pclass_id);
+JS_EXTERN JSClassID JS_NewClassID(JSRuntime *rt);
 /* Returns the class ID if `v` is an object, otherwise returns JS_INVALID_CLASS_ID. */
 JS_EXTERN JSClassID JS_GetClassID(JSValue v);
 JS_EXTERN int JS_NewClass(JSRuntime *rt, JSClassID class_id, const JSClassDef *class_def);