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: Fix accessing Frame from multiple Threads by making it Thread-safe #2327

Merged
merged 16 commits into from
Dec 29, 2023

Conversation

mrousavy
Copy link
Owner

@mrousavy mrousavy commented Dec 29, 2023

What

Accessing Frame from multiple Threads was unsafe. The Frame could've been deallocated by now.

With this PR, we check if the Frame is still valid and execute all unsafe methods under a synchronized block/Lock to make sure the app doesn't crash with null-unwrap errors.

frame.isValid

The only property that will not throw when trying to access a closed frame is isValid:

console.log(frame.isValid)
false

If you access any other property (e.g. width) on a closed Frame, it will throw an error:

console.log(frame.width)
Error: Exception in HostObject::get for prop 'width': Frame is already closed! Are you trying to access the Image data outside of a Frame Processor's lifetime? If yes, increment it's ref-count: `frame.incrementRefCount()`, js engine: hermes

console.log(frame)

Since console.log is a function defined on the JS Thread only, calling it inside a Frame Processor will require a Thread hop - essentially doing a runOnJS/Worklets.createRunInJSFunc. When you pass the frame to that function, it will transfer the Frame over to the JS Thread, and try to log the properties of the Frame later. The problem here is, that by the time the console.log function tries to log the frame, the frame will already be deallocated since the Frame Processor has already finished it's execution in the meantime and a new Frame arrives.

So this will throw an error:

console.log(frame)

Because the frame will be no longer valid when console.log runs.

Instead, try this to debug Frames:

console.log(frame.toString())

Java/Kotlin Changes

The Java/Kotlin Frame type brings two changes:

  • frame.orientation: This is no longer just a String, but instead the actual type-safe Orientation enum. This improves developer experience for native Java/Kotlin Frame Processor Plugins
  • frame.pixelFormat: This is no longer just a String, but instead the actual type-safe PixelFormat enum. This improves developer experience for native Java/Kotlin Frame Processor Plugins

Changes

Tested on

Related issues

Copy link

vercel bot commented Dec 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 29, 2023 1:09pm

@mrousavy mrousavy merged commit 895f3ec into main Dec 29, 2023
12 checks passed
@mrousavy mrousavy deleted the fix/logging branch December 29, 2023 13:09
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.

None yet

1 participant