Skip to content

fix: Clear native output callbacks in output hooks on unmount - #1

Closed
Pap36 wants to merge 1 commit into
mainfrom
fix/clear-output-callbacks-on-unmount
Closed

Pap36 wants to merge 1 commit into
mainfrom
fix/clear-output-callbacks-on-unmount

Conversation

@Pap36

@Pap36 Pap36 commented Sep 14, 2026

Copy link
Copy Markdown
Owner

What

useFrameOutput, useDepthOutput and useObjectOutput hand JS callbacks to a native output (setOnFrameCallback, setOnFrameDroppedCallback, and their depth/object equivalents) and never clear them. Nitro keeps those callbacks alive as GC roots for as long as the native output exists, and the callbacks close over the hook's frameOutput and its per-mount worklet runtime. That is a cycle across the JSI boundary, so no garbage collection can ever break it: every mount of a component using useFrameOutput leaks a full worklet runtime plus whatever the onFrame worklet captured.

In a dev build with Worklets Bundle Mode each leaked runtime holds its own compiled copy of the app bundle. On an iPhone 11 our scanner screen cost ~350 MB per open and nothing came back on unmount, so the second open hit the Jetsam limit. Release builds leak the same objects, just far smaller (bytecode is mmapped and shared), so it shows up as slow growth instead of a crash.

How

Return effect cleanups from the three hooks that set the native callbacks back to undefined. That is the same call the Harness tests already make in their finally blocks. Clearing on unmount (and on callback change, before the new one is set) lets the frame output, the runtime and the captured scope be collected normally.

Verified on device (iPhone 11, iOS 26.6.1, dev client, physFootprint sampled every 3 s):

Step Before After cleanup
Home screen baseline 1509 MB 1509 MB
First open of the frame-processor screen 1622 MB 1622 MB
Back 1636 MB, nothing freed 1631 MB
Forced GC (DevTools) no change −324 MB
Second open 1931 MB → Jetsam kill survives

A WeakRef probe on the hook's runtime and frameOutput confirmed the mechanism: both stayed alive across a forced full GC before the change, and the runtime was collected after it. The heap snapshot retainer path goes (GC roots) → (Custom) → onFrameDropped closure → hook environment → frameOutput / runtime.

useDepthOutput and useObjectOutput have the identical pattern and get the same cleanup.

Test

Adds a Harness test to visioncamera.hooks.harness.tsx that mounts a component calling useFrameOutput, attaches its output to a CameraSession created outside React (so the pipeline outlives the component), waits for Frames, unmounts, then captures two photos as the pipeline clock and asserts that no further Frames reached the unmounted onFrame.

Ran locally on a Pixel 7 (Android 17) with bun run test:harness:android -- --testPathPatterns=hooks:

  • With the fix: all 7 tests in the file pass (1 pre-existing skip), the new one in ~1.2 s.

  • With main's three hook files restored and the test kept: the new test fails at expect(onFrameReceived).toHaveBeenCalledTimes(framesAfterUnmount), because Frames keep arriving after unmount(). In one of the two control runs the app instead died on the frame thread with

    FATAL EXCEPTION: com.margelo.camera.frame
    java.lang.RuntimeException: Cannot call SyncJSCallback<bool (std::shared_ptr<HybridFrameSpec> const&)> - the underlying `jsi::Function` has already been deleted!
        at com.margelo.nitro.camera.hybrids.outputs.HybridFrameOutput.updateAnalyzer$lambda$4(HybridFrameOutput.kt:126)
    

    That is the Android face of the same bug: setOnFrameDroppedCallback is a no-op there, so nothing pins the runtime, the GC eventually frees the worklet runtime, and CameraX's analyzer still calls into the deleted jsi::Function on the next Frame. On iOS the drop callback pins everything and it leaks instead. Clearing the analyzer on unmount fixes both.

Related

@Pap36
Pap36 force-pushed the fix/clear-output-callbacks-on-unmount branch from 4ff63cc to cf5f16a Compare September 14, 2026 14:02
useFrameOutput, useDepthOutput and useObjectOutput hand JS callbacks to
native outputs and never clear them. The native side pins those callbacks
as GC roots, so the hook's frame output, its worklet runtime and everything
the callbacks close over stay alive after the component unmounts.

Return effect cleanups that set the callbacks back to undefined, and add a
Harness test that mounts useFrameOutput on a session which outlives the
component and asserts no further Frames reach the unmounted callback.
@Pap36
Pap36 force-pushed the fix/clear-output-callbacks-on-unmount branch from cf5f16a to a359ab7 Compare September 14, 2026 14:15
@Pap36

Pap36 commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Preview only; submitted upstream as margelo#4192.

@Pap36 Pap36 closed this Sep 14, 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.

1 participant