• JS runtime-pool trigger thread null-derefs and crashes whole process w

    From Rob Swindell@1:103/705 to GitLab issue in main/sbbs on Mon Jun 1 23:15:51 2026
    open https://gitlab.synchro.net/main/sbbs/-/issues/1152

    ## Summary

    The JS runtime-pool "trigger" thread (`src/sbbs3/js_rtpool.cpp`) can dereference a **NULL `JSRuntime*`** and crash the entire process (an access violation inside `JS_TriggerAllOperationCallbacks`). It happens when `JS_NewRuntime()` fails — e.g. under memory pressure — because the failed (NULL) runtime is added to the pool's list unchecked, and the trigger thread then walks the list and calls into the JS engine with it.

    Because all servers run in-process under `sbbsctrl.exe` (and `sbbscon`), this one bad allocation in a single web-request path takes the **whole BBS** down.

    ## Observed crash

    `sbbsctrl.exe` (v3.21.4.0, 32-bit, mozjs185 1.8.5) crashed on a production system twice (2026-05-29, 2026-06-01), identical WER bucket. Minidump analysis (cdb):

    ```
    mozjs185_1_0!JS_TriggerAllOperationCallbacks+0x5:
    mov eax, dword ptr [esi+164h] ; esi (JSRuntime*) = 00000000 -> read of 0x00000164
    ExceptionCode: c0000005 (Access violation, read)

    STACK:
    mozjs185_1_0!JS_TriggerAllOperationCallbacks+0x5
    sbbs!thread_start<void (__cdecl*)(void *),0>+0xb8 ; == js_rtpool.cpp trigger_thread
    kernel32!BaseThreadInitThunk
    ntdll!_RtlUserThreadStart
    ```

    Each crash was preceded (same minute) by web-log entries indicating the process was out of JS heap, e.g.:

    ```
    web ... !JavaScript webfileindex.ssjs line 299: out of memory, Request: /files/...?view=...
    web ... JavaScript: Failed to create new context
    ```

    i.e. memory pressure → `JS_NewRuntime()` returns NULL → crash.

    ## Root cause

    `src/sbbs3/js_rtpool.cpp`:

    ```cpp
    static void trigger_thread(void *args) // 100ms loop, forever
    {
    ...
    for (node = listFirstNode(&rt_list); node; node = listNextNode(node))
    JS_TriggerAllOperationCallbacks(static_cast<JSRuntime *>(node->data)); // derefs node->data
    ...
    }

    JSRuntime * jsrt_GetNew(int maxbytes, ...)
    {
    ret = JS_NewRuntime(maxbytes); // can return NULL on failure
    listPushNode(&rt_list, ret); // <-- NULL pushed onto the list unchecked
    ...
    }
    ```

    `JS_NewRuntime()` returning NULL is a documented, expected failure mode (out of memory). Listing a NULL runtime guarantees the trigger thread will dereference it within ~100ms.

    This is platform-agnostic (the pool/trigger logic is shared); it was just observed first on the 32-bit Windows build, which is most prone to running the JS heap dry. Latent since the runtime-pool list rework in `4173ce48d0` (2014).

    ## Fix

    Don't list a NULL runtime, and have the trigger thread skip NULLs defensively:

    ```cpp
    ret = JS_NewRuntime(maxbytes);
    if (ret != NULL)
    listPushNode(&rt_list, ret);
    ```
    ```cpp
    for (node = listFirstNode(&rt_list); node; node = listNextNode(node))
    if (node->data != NULL)
    JS_TriggerAllOperationCallbacks(static_cast<JSRuntime *>(node->data)); ```

    A fix along these lines is prepared and builds clean (MSVC Win32 Release). Note this addresses the *crash*; the underlying memory pressure that makes `JS_NewRuntime()` fail (a crawler walking the dynamic file index) is a separate, mitigated concern.

    — *Authored by Claude (Claude Code), on behalf of @rswindell*
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell@1:103/705 to GitLab note in main/sbbs on Mon Jun 1 23:27:10 2026
    https://gitlab.synchro.net/main/sbbs/-/issues/1152#note_9143

    Fixed in **fab8b1f1d** (`js_rtpool: don't crash when JS_NewRuntime() fails (NULL runtime)`).

    The committed fix is three NULL guards in `src/sbbs3/js_rtpool.cpp` (the description above listed only the first two — the third closes a sibling hole found in review):

    ```cpp
    // jsrt_GetNew(): don't list a NULL runtime (the actual crash fix)
    ret = JS_NewRuntime(maxbytes);
    if (ret != NULL)
    listPushNode(&rt_list, ret);
    ```
    ```cpp
    // trigger_thread(): skip a NULL node (defense-in-depth at the crash site)
    for (node = listFirstNode(&rt_list); node; node = listNextNode(node))
    if (node->data != NULL)
    JS_TriggerAllOperationCallbacks(static_cast<JSRuntime *>(node->data)); ```
    ```cpp
    // jsrt_Release(): no-op on NULL (JS_DestroyRuntime(NULL) would crash the same // way; not currently reachable since all 7 callers bail on a NULL GetNew, but // same bug class)
    if (rt == NULL)
    return;
    ```

    Builds clean under MSVC Win32 Release. Not yet runtime-tested — exercising it requires actually driving `JS_NewRuntime()` to fail under memory pressure (the rare condition behind the crash); the changes are plain null guards with no behavioral change in the normal path.

    The memory pressure that made `JS_NewRuntime()` fail was a crawler walking the dynamic file index (`webfileindex.ssjs` materializes the whole `file_area` per request). That's mitigated separately in **d777bda5c** (`webfileindex: deflect crawlers from ?view=`), which returns 429 to known crawler User-Agents on `?view=` requests. That's a load-reducer, not the crash fix — fab8b1f1d is what actually prevents the crash.

    — *Authored by Claude (Claude Code), on behalf of @rswindell*
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell@1:103/705 to GitLab note in main/sbbs on Mon Jun 1 23:55:42 2026
    https://gitlab.synchro.net/main/sbbs/-/issues/1152#note_9148

    Deployed: the fixed `sbbs.dll` (commit fab8b1f1d) is now built and running on the affected system. Closing as resolved.

    — *Authored by Claude (Claude Code), on behalf of @rswindell*
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell@1:103/705 to GitLab issue in main/sbbs on Mon Jun 1 23:55:42 2026
    close https://gitlab.synchro.net/main/sbbs/-/issues/1152
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)