Skip to content

Bind async work to its JS runtime generation - #435

Closed
babzcraig wants to merge 8 commits into
OP-Engineering:mainfrom
babzcraig:tfl/js-runtime-generation-safety
Closed

Bind async work to its JS runtime generation#435
babzcraig wants to merge 8 commits into
OP-Engineering:mainfrom
babzcraig:tfl/js-runtime-generation-safety

Conversation

@babzcraig

Copy link
Copy Markdown
Contributor

Follow-up to #434. This is the per-generation liveness work you asked for, plus the smaller teardown races we found alongside it.

Your two commits from #434 are cherry-picked underneath, unchanged and still authored by you, so this branch applies to main on its own. If you would rather merge #434 first, do that and I will drop them and rebase.

The problem

React Native's bridgeless reload does not tear one runtime down and then build the next. RCTHost constructs the new RCTInstance while the old one is still invalidating, so two generations overlap. The process globals opsqlite::invoker and opsqlite::invalidated cannot describe that: whichever generation wrote last wins. A database created by the outgoing runtime would read the global invoker when its work completed, post into the runtime that replaced it, and call asFunction() on a jsi::Value owned by the dead one.

The commits

  • Bind hooks and reactive flush to their runtime generation. Adds opsqlite::generation_alive, a shared_ptr<atomic<bool>> replaced by install() and cleared by invalidate(). DBHostObject captures the invoker and that flag at construction on the JS thread. They shadow the globals inside member functions, so every invokeAsync site in the class is fixed without editing each one.
  • Bind promisify tasks to their runtime generation. Same fix on the thread pool: the invoker and flag are captured while the promise is constructed rather than read when the task finishes, and the early return tests this generation instead of the global.
  • Interrupt in-flight queries before draining on invalidate. invalidate() waited on the pool but never called sqlite3_interrupt, unlike close() and delete(). React Native abandons module invalidation after ten seconds and destroys the runtime anyway, so an uninterrupted drain can lose that race.
  • Guard the database registry with a mutex. open() runs on the JS thread and invalidate() on the TurboModule queue, and during a reload those overlap, so both can reach dbs at once. invalidate() now swaps the vector out under the lock and works on its own copy.
  • Wake all thread pool waiters and make done atomic. waitFinished() and doWork() share one condition variable, so notify_one() can deliver the wakeup to the wrong waiter. done is read outside the mutex while the destructor writes it.
  • Never let a pool thread join itself. A backstop for join() throwing "Resource deadlock avoided" if a future owner releases the pool on one of its own workers. Dropping the pool capture in Patches around async work #434 removes the known route there.

Deliberately left out

Our patch also bounds the drain (waitFinished(timeout)) and, on timeout, leaks the pool and the sqlite handle rather than freeing them underneath a live worker. That is a policy call about what to do when a query will not stop, and it did not belong inside a crash fix, so it is not here. Happy to open it separately if you want it.

Known limit

The early return in promisify still drops the last references on the pool thread, so ~jsi::Value runs there rather than on the JS thread. That behaviour is pre-existing, and the sqlite3_interrupt change makes it very hard to reach, but it is not airtight without either leaking the values or handing them to the JS thread.

Also worth noting: after this change, opsqlite::invalidated is written but never read, since promisify was its only reader.

ospfranco and others added 8 commits August 13, 2026 00:34
During a bridgeless reload two JS runtime generations overlap: the outgoing
one is invalidated while the incoming one is already installing. The process
globals invoker and invalidated cannot describe that, so a database created by
the outgoing generation would post its hook and reactive-query callbacks into
the runtime that replaced it, and then call asFunction() on a jsi::Value owned
by the dead one.

Each generation now gets its own liveness flag, and DBHostObject binds both the
invoker and that flag at construction, on the JS thread. The members shadow the
globals inside member functions, so every invokeAsync site is fixed without
touching each call site.
Same problem as the hooks, on the thread pool. A task read opsqlite::invoker
when it completed rather than when it was queued, so a query started by the
outgoing runtime could resolve into the incoming one and call asFunction() on a
jsi::Value that belongs to the dead runtime.

The invoker and the liveness flag are now captured on the JS thread while the
promise is constructed, and the early return checks this generation rather than
the process-global invalidated, which is set by whichever generation tore down
last.

Note the early return still drops the last references on the pool thread, so
~jsi::Value runs there. That is pre-existing, and the sqlite3_interrupt change
that follows makes it very hard to reach, but it is not airtight without either
leaking the values or handing them to the JS thread.
invalidate() waits on the thread pool but, unlike close() and delete(), never
asks SQLite to stop. A query inside sqlite3_step therefore runs to completion
while the JS runtime is being torn down. React Native gives module invalidation
ten seconds before it destroys the runtime regardless, so an uninterrupted
drain can lose that race and leave a worker touching state the runtime owned.

This is the same three lines close() already runs, moved to the one teardown
path that lacked them.
open() runs on the JS thread and invalidate() runs on the TurboModule queue.
During a bridgeless reload those belong to different generations and overlap,
because the new instance is constructed before the old one finishes
invalidating, so both can reach the dbs vector at once. Iterating it while
another thread emplaces can walk a reallocated buffer.

invalidate() now swaps the vector out under the lock and works on its own copy,
which also keeps the lock off the invalidate() calls themselves.
waitFinished() and doWork() wait on the same condition variable, so
notify_one() can wake a worker when the waiter needed the wakeup, or the other
way round, and leave the other one asleep until the next event. With a single
pool thread that is easy to hit at teardown, where waitFinished() is the only
waiter that matters.

done is read in the doWork() loop condition outside the mutex while the
destructor writes it, which is a data race; make it atomic.
If the last shared_ptr to a ThreadPool is released on one of its own workers,
the destructor tries to join the running thread and std::thread::join throws
"Resource deadlock avoided", which aborts. Dropping the pool capture from the
promisify task removes the known way to get there, so this is a backstop for
any future owner that ends up released on a worker.
@ospfranco

Copy link
Copy Markdown
Contributor

Going to close this as I cleaned up a bit the code in #436 (and it's stack). I will still credit you :)

Moving some of the comments there

@ospfranco ospfranco closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants