[camera_android_camerax] Fix leaked thread per capture in takePicture - #12361
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses a thread leak in the camera_android_camerax package by dispatching the takePicture result callback on the main executor instead of a new single-thread executor. Unit tests in ImageCaptureTest.java have been added and updated to verify this change. There are no review comments, so I have no feedback to provide.
camsim99
left a comment
There was a problem hiding this comment.
Thanks for filing a clear issue and the quick fix! This totally makes sense to me.
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
|
autosubmit label was removed for flutter/packages/12361, because Pull request flutter/packages/12361 is not in a mergeable state. |
Fixes flutter/flutter#190523 `ImageCaptureProxyApi.takePicture` creates a new single-thread executor on every call and never shuts it down. The thread it owns is only reclaimed once its `FinalizableDelegatedExecutorService` wrapper is finalized, at a time the plugin does not control. That executor only delivers an already finished result — it does no real work: - Encoding and file I/O run on CameraX's own IO executor. - `TakePictureRequest.onResult` does nothing but hand the result to the executor the plugin passed in. Dispatch the result on the main executor instead, which is what this plugin's other CameraX callbacks already use: - `CameraControlProxyApi` - `Camera2CameraControlProxyApi` - `ImageAnalysisProxyApi` - `ProcessCameraProviderProxyApi` - `PendingRecordingProxyApi` `PreviewProxyApi` uses the same per-request executor pattern for `SurfaceRequest.provideSurface`. That callback releases a `Surface`, so it is left for a separate change. The two existing `takePicture` tests now stub the main executor on their mock `Context`.
6a2806f to
c85072a
Compare
Fixes flutter/flutter#190523
Problem
ImageCaptureProxyApi.takePicturecreates a new single-thread executor onevery call and never shuts it down. The thread it owns is only reclaimed
once its
FinalizableDelegatedExecutorServicewrapper is finalized, at atime the plugin does not control.
That executor only delivers an already finished result — it does no real
work:
TakePictureRequest.onResultdoes nothing but hand the result to theexecutor the plugin passed in.
Fix
Dispatch the result on the main executor instead, which is what this
plugin's other CameraX callbacks already use:
CameraControlProxyApiCamera2CameraControlProxyApiImageAnalysisProxyApiProcessCameraProviderProxyApiPendingRecordingProxyApiNote
PreviewProxyApiuses the same per-request executor pattern forSurfaceRequest.provideSurface. That callback releases aSurface, so itis left for a separate change.
The two existing
takePicturetests now stub the main executor on theirmock
Context.Pre-Review Checklist
[shared_preferences]///).