fix: [Apple] notify the texture registry on the main thread - #1769
fix: [Apple] notify the texture registry on the main thread#1769luutruong wants to merge 2 commits into
Conversation
|
Bug reported: #1768 |
2f9d9a4 to
228e45b
Compare
navaronbracke
left a comment
There was a problem hiding this comment.
LGTM, although this will need a patch version bump and changelog entry
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1769 +/- ##
========================================
Coverage 60.40% 60.40%
========================================
Files 48 48
Lines 1225 1225
========================================
Hits 740 740
Misses 485 485
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@luutruong Friendly ping for the patch version and changelog entry |
Thank you. I've run this patch on my app in production. So far, crash is gone. |
`captureOutput(_:didOutput:from:)` runs on `sampleBufferQueue` and called `registry.textureFrameAvailable` directly from that queue. The Flutter engine is torn down synchronously on the main thread, so an in-flight frame callback could call into `-[FlutterEngine textureFrameAvailable:]` while the engine was already mid-`dealloc`, crashing with EXC_BAD_ACCESS. `FlutterTextureRegistryRelay` holds its parent weakly, but a weak reference is only cleared after `dealloc` completes, so it does not guard against a call that arrives during teardown. Hop the notification to the main queue so it is serialized against the teardown by the queue itself, rather than relying on notification or plugin-detach ordering, and drop frames whose texture was released while the block was queued. Also implement `detachFromEngineForRegistrar:` to release the camera and texture deterministically when the engine detaches the plugin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
228e45b to
f8d849e
Compare
|
@juliansteenbakker do we merge this into 7.4.1? Since I see that release is still pending |
|
Thanks for the PR! I've landed the fix in #1797, which takes the same main queue approach but coalesces the notification so we don't queue a block per frame, and stops the camera on willTerminate rather than relying on the relay's weak parent, which doesn't help here since the engine object outlives its shell. |
Problem
MobileScannerPlugin.captureOutput(_:didOutput:from:)runs on the plugin's ownsampleBufferQueue(com.juliansteenbakker.mobile_scanner.captureOutputQueue) and callsregistry.textureFrameAvailable(textureId)directly from that queue.The Flutter engine, however, is destroyed synchronously on the main thread
(
-[FlutterViewController appOrSceneWillTerminate]→-[FlutterEngine destroyContext]→Shell::~Shell). If a frame callback is in flight on the capture queue at that moment, it calls-[FlutterEngine textureFrameAvailable:]on an engine that is already mid-dealloc, with apartially destroyed
Shell→EXC_BAD_ACCESS.FlutterTextureRegistryRelayforwards to a weakparent, but a weak reference is only clearedafter
dealloccompletes, so it does not protect against a call that arrives during teardown.We see this in production on physical iPhones, release builds, on the order of hundreds of crashes
per day for a screen that keeps a scanner running.
Crash stack (redacted)
Steps to reproduce
MobileScannerwidget with the camera running.Why a Dart-side workaround cannot fix this
Stopping the camera from
didChangeAppLifecycleState(inactive/paused/detached)does not help: itneeds a platform-channel round trip, while termination is synchronous inside the notification
callout and the Dart isolate is shut down in that same call chain (
Dart_ShutdownIsolateis visiblein the stack above). The stop message is never delivered.
Stopping the session natively is not sufficient on its own either —
captureSession.stopRunning()does not drain a callback that is already executing on the capture queue.
Fix
Hop the texture notification to the main queue. Since engine teardown also runs on the main thread,
the queue itself serializes the two, so correctness no longer depends on notification or
plugin-detach ordering. A block enqueued after the engine is gone finds the relay's parent already
nil and becomes a no-op, and the
textureId != nilre-check drops frames whose texture was releasedwhile the block was queued.
Barcode analysis stays off the main thread exactly as before — only the registry notification moves,
which just marks the texture dirty and schedules a frame.
Additionally,
detachFromEngineForRegistrar:is now implemented to release the camera and texturedeterministically when the engine detaches the plugin (defense in depth; the main-queue hop is what
actually closes the race).
Not touched: the separate
latestBuffer/copyPixelBuffer()interaction with the raster thread —out of scope for this fix.
Notes
No changelog entry added, since the changelog is generated from the Conventional Commit PR title.
Happy to adjust the approach — for example coalescing pending notifications if the extra main-queue
hop per frame is a concern on lower-end devices.
Fixes [iOS/macOS] captureOutput calls textureFrameAvailable from a background queue → use-after-free crash on engine teardown #1768