Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Threading issues (SV access from different Thread) #1883

Merged

Conversation

mrousavy
Copy link
Contributor

@mrousavy mrousavy commented Mar 31, 2021

Description

There are a few functions that check if you're currently on the REA UI Runtime by using RuntimeDecorator::isWorkletRuntime. This might actually lead to threading issues if you're calling those functions from another worklet runtime that is not from Reanimated (e.g. VisionCamera Frame Processors, Multithreading, ...) - so this PR fixes this. A few of those functions should actually check if they are specifically on the UI thread, Worklet thread is not enough.

Changes

  • Add RuntimeDecorator::isUIRuntime - a function that specifically checks if the given jsi::Runtime is the REA UI Runtime as opposed to any Worklet Runtime
  • Replace a few isWorkletRuntime calls with isUIRuntime

Test code and steps to reproduce

Checklist

  • Ensured that CI passes

@mrousavy mrousavy marked this pull request as ready for review May 1, 2021 14:06
@mrousavy
Copy link
Contributor Author

mrousavy commented May 1, 2021

Okay so I've managed to verify that with the current REA codebase, assigning or reading a SharedValue in a worklet (on a separate runtime, such as in react-native-multithreading or react-native-vision-camera frame processors) crashes because of a threading issue.

This was not an issue before since no other library spawns "worklet" runtimes.

This PR fixes this by using isUIRuntime where we need the UI thread, and isWorkletRuntime where we need any runtime that supports running "worklets", and isReactRuntime for explicitly checking if it is the default React-JS runtime.

No Reanimated behaviour has changed, this only affects libraries such as rn-multithreading or react-native-vision-camera 👍

cc @Szymon20000 could you take a look at this when you have some free time? ❤️ 🙏

@mrousavy mrousavy changed the title Fix runtime scheduling Fix Threading issues (SV access from different Thread) May 3, 2021
@Szymon20000 Szymon20000 merged commit c511a5d into software-mansion:master May 4, 2021
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