Skip to content

fix(Worklets): time-of-check and time-of-use destructor safety#9790

Merged
tjzel merged 9 commits into
mainfrom
@tjzel/worklets/toctou
Jul 1, 2026
Merged

fix(Worklets): time-of-check and time-of-use destructor safety#9790
tjzel merged 9 commits into
mainfrom
@tjzel/worklets/toctou

Conversation

@tjzel

@tjzel tjzel commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Currently the way we check if the runtime is alive for safe destructor invocations has a chance to lead to crashes anyway, since the runtime can get cleaned up immediately after the check, since it lacks a locking mechanism.

Test plan

The test example from #9789 works.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens destructor-time cleanup of JSI values by eliminating a time-of-check/time-of-use window when deciding whether it’s safe to run JSI destructors while a runtime may be concurrently tearing down.

Changes:

  • Replace atomic “is dead” checks with mutex-protected, locked execution for RN runtime status.
  • Update SerializableRemoteFunction and generic cleanupRuntimeAware cleanup paths to perform “check + destroy” while holding the relevant lifetime lock.
  • Refactor WorkletRuntimeRegistry from a boolean query API to a runWhileLocked API to support atomic check+action.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/react-native-worklets/Common/cpp/worklets/Tools/RNRuntimeStatus.h Switches runtime-dead tracking from atomic reads to mutex-locked execution.
packages/react-native-worklets/Common/cpp/worklets/SharedItems/SerializableRemoteFunction.cpp Uses the new locked runtime-status path to safely reset/free remote functions during destruction.
packages/react-native-worklets/Common/cpp/worklets/SharedItems/Serializable.h Updates runtime-aware cleanup to use registry-locked “alive check + destroy” semantics.
packages/react-native-worklets/Common/cpp/worklets/Registries/WorkletRuntimeRegistry.h Replaces isRuntimeAlive with a runWhileLocked helper to prevent TOCTOU.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/react-native-worklets/Common/cpp/worklets/Tools/RNRuntimeStatus.h Outdated
@tjzel tjzel marked this pull request as ready for review June 29, 2026 21:22
Base automatically changed from @tjzel/worklets/keep-rn-runtime-status to main July 1, 2026 08:09
@tjzel tjzel changed the title fix: time-of-check and time-of-use destructor safety fix(Worklets): time-of-check and time-of-use destructor safety Jul 1, 2026
@tjzel tjzel added this pull request to the merge queue Jul 1, 2026
Merged via the queue into main with commit 71774cb Jul 1, 2026
12 checks passed
@tjzel tjzel deleted the @tjzel/worklets/toctou branch July 1, 2026 08:47
tjzel added a commit that referenced this pull request Jul 1, 2026
## Summary

Currently the way we check if the runtime is alive for safe destructor
invocations has a chance to lead to crashes anyway, since the runtime
can get cleaned up immediately after the check, since it lacks a locking
mechanism.

## Test plan

The test example from #9789 works.

(cherry picked from commit 71774cb)
tjzel added a commit that referenced this pull request Jul 1, 2026
## Summary

Currently the way we check if the runtime is alive for safe destructor
invocations has a chance to lead to crashes anyway, since the runtime
can get cleaned up immediately after the check, since it lacks a locking
mechanism.

## Test plan

The test example from #9789 works.

(cherry picked from commit 71774cb)
tjzel added a commit that referenced this pull request Jul 14, 2026
## Summary

Currently the way we check if the runtime is alive for safe destructor
invocations has a chance to lead to crashes anyway, since the runtime
can get cleaned up immediately after the check, since it lacks a locking
mechanism.

## Test plan

The test example from #9789 works.

(cherry picked from commit 71774cb)
tjzel added a commit that referenced this pull request Jul 14, 2026
## Summary

Currently the way we check if the runtime is alive for safe destructor
invocations has a chance to lead to crashes anyway, since the runtime
can get cleaned up immediately after the check, since it lacks a locking
mechanism.

## Test plan

The test example from #9789 works.

(cherry picked from commit 71774cb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants