facebook / facebook/hermes

No-pthread Emscripten runtime throws during JSI finalization via SerialExecutor

Open
#2,125 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
11.3k
Forks
859
Avg merge
1h 30m
Merged PRs (30d)
3

Description

## Bug Description

Hermes's documented Emscripten and Static Hermes AOT build flow does not enable pthreads, but `SerialExecutor` selects `std::thread` for every Emscripten build. When an Emscripten runtime containing JSI host functions is destroyed, host-function finalization submits deletion to `SerialExecutor`; constructing its worker thread then throws because the module was built without Emscripten pthread support.

I observed the failure on `static_h` revision `47b8dbab74e308e85a31165f51748c2393340ca1`. The relevant implementation is unchanged at current `static_h` revision `a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9`.

Hermes git revision: `a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9` (source inspection); originally reproduced at `47b8dbab74e308e85a31165f51748c2393340ca1`

React Native version: not applicable

OS: Linux x86_64

Platform: `wasm32-unknown-emscripten`, Emscripten 6.0.5, no `-pthread`

The documented build command in [`doc/Emscripten.md`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/doc/Emscripten.md#compile-the-hermes-vm-to-wasm) and the AOT link command in [`utils/wasm-compile.sh`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/utils/wasm-compile.sh) do not enable pthreads.

## Steps To Reproduce

1. Build the host tools and Emscripten libraries using the commands in `doc/Emscripten.md`, without adding `-pthread`.
2. Compile and link a Static Hermes unit that creates a JSI host function. The built-in/embedding host function can be minimal; its context only needs to survive until runtime finalization.
3. Execute the unit and destroy the Hermes runtime, allowing Hades to finalize the host function.
4. Observe thread construction from `SerialExecutor::add` during teardown.

The relevant teardown path is:

```text
HermesRuntimeImpl::HFContext::finalize
FinalizableNativeFunction::_finalizeImpl
HadesGC::finalizeAll
HermesRuntimeImpl::~HermesRuntimeImpl
SerialExecutor::add
std::thread construction
```

`HFContext::finalize`, `JsiProxy`, and `NativeStateContext` all submit deletion to `finalizerExecutor_`. The executor is intentionally described as background cleanup so GC does not block:

- [`API/hermes/hermes.cpp`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/API/hermes/hermes.cpp#L929-L936)
- [`API/hermes/hermes.cpp`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/API/hermes/hermes.cpp#L1095-L1099)
- [`API/hermes/hermes.cpp`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/API/hermes/hermes.cpp#L1114-L1118)
- [`API/hermes/hermes.cpp`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/API/hermes/hermes.cpp#L1355-L1362)

For Emscripten, `SerialExecutor` stores a `std::thread` and its `add` path constructs that thread without checking whether pthreads were enabled:

- [`include/hermes/Support/SerialExecutor.h`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/include/hermes/Support/SerialExecutor.h#L41-L46)
- [`lib/Support/SerialExecutor.cpp`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/lib/Support/SerialExecutor.cpp#L45-L72)

There is already a no-thread Wasm fallback in `StackExecutor`, although it uses the different `__EMSCRIPTEN_THREADS__` feature macro:

- [`lib/Support/StackExecutor.cpp`](https://github.com/facebook/hermes/blob/a7086fcea2dd36e3d9475bd1ec8fb2f035d911b9/lib/Support/StackExecutor.cpp#L87-L105)

## Expected Behavior

A Hermes runtime built with the documented no-pthread Emscripten configuration should not attempt to create a guest thread during finalization. Runtime destruction should complete normally, or an unsupported executor-dependent feature should be rejected explicitly before execution rather than throwing from teardown.

## Workaround and why it may not be the right general fix

This narrow workaround makes the failing finalizer path complete:

```cpp
#if defined(__EMSCRIPTEN__) && !defined(__EMSCRIPTEN_PTHREADS__)
task();
return;
#endif
```

It is not behavior-preserving enough to propose as the final fix without maintainer guidance. `SerialExecutor` normally provides queued worker execution. Running every task inline changes:

- FIFO behavior for nested `add` calls: `A -> add(B)` becomes reentrant `A -> B -> A`;
- exception propagation, because exceptions now escape `add` on the caller's stack;
- thread affinity and caller latency;
- teardown checks, timeout behavior, and stack-size behavior; and
- semantics for other users such as NAPI async work, not only finalization.

Inline host-object or host-function deletion can also execute arbitrary destructor work while GC or runtime teardown is active, whereas the existing code deliberately moves that work to a background thread.

## Possible Fix Directions

I would appreciate guidance on which contract Hermes wants for Emscripten without pthreads. Some possible directions are:

1. **Explicit no-thread `SerialExecutor` backend.** Queue tasks and drain them synchronously with a reentrancy guard, defining FIFO, nested-add, exception, and destruction behavior. This is small, but finalizer work still runs on the caller's thread.
2. **Host-provided scheduler/executor.** Allow the embedding to schedule finalizer work through its event loop. This best preserves deferred execution but expands the embedding contract and needs a shutdown/drain protocol.
3. **Finalizer-specific no-thread queue.** Keep `SerialExecutor`'s worker-thread contract intact, but make `HermesRuntimeImpl` collect finalizer deletions and drain them at explicit safe boundaries or during ordered teardown. This avoids silently changing NAPI/CDP executor behavior.
4. **Feature gating.** If some executor users fundamentally require threads, reject or disable those features in no-pthread builds while providing a separately defined strategy for mandatory runtime finalization.

Whichever direction is selected, useful tests appear to include:

- HostFunction, HostObject, and NativeState cleanup in an Emscripten no-pthread build;
- nested task submission and FIFO/reentrancy behavior;
- task exceptions;
- runtime destruction with pending cleanup; and
- explicit behavior for NAPI or other components requiring a real worker.

Is no-pthread Emscripten execution intended to cover this JSI finalization path, and if so, which scheduling semantics would be preferred?

Contributor guide

Open the contributing guide

Research direction

Start with include/hermes/Support/SerialExecutor.h and lib/Support/SerialExecutor.cpp, then trace the finalization callers in API/hermes/hermes.cpp. Reproduce runtime destruction using the no-pthread commands in doc/Emscripten.md and utils/wasm-compile.sh. Done should mean HostFunction, HostObject, and NativeState cleanup completes without guest-thread creation, with the selected scheduling behavior covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, javascript, wasm
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.