Skip to content

[camera_android] Avoid thread race conditions when closing the camera. #9090

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/camera/camera_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.10.10+3

* Fixes thread race condition when closing the camera.

## 0.10.10+2

* Don't set the FPS range unless video recording. It can cause dark image previews on some devices becuse the auto exposure algorithm is more constrained after fixing the min/max FPS range to the same value. This change has the side effect that providing the `fps` parameter will not affect the camera preview when not video recording. And if you need a lower frame rate in your image streaming handler, you can skip frames by checking the time it passed since the last frame.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1293,10 +1293,12 @@ void setImageStreamImageAvailableListener(final EventChannel.EventSink imageStre
}

void closeCaptureSession() {
if (captureSession != null) {
// Keep a local copy to avoid race conditions between threads.
final CameraCaptureSession captureSessionToClose = captureSession;
if (captureSessionToClose != null) {
Log.i(TAG, "closeCaptureSession");

captureSession.close();
captureSessionToClose.close();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this fix the race condition? If I'm understanding correctly, the premise is that we are calling this method simultaneously from two different threads. If that's true, then this version will call close twice on the same session, which will almost certainly just replace a null pointer exception with state error exception.

If this class is trying to interact with the same objects from multiple threads at the same time, that needs to actually be fixed in some robust way (most likely either locking or cross-thread redispatch). Changing individual variable access is almost never the right way to handle threading issues, and that seems to apply here as well.

captureSession = null;
}
}
Expand Down Expand Up @@ -1324,8 +1326,10 @@ public void close() {
}

private void stopAndReleaseCamera() {
if (cameraDevice != null) {
cameraDevice.close();
// Keep a local copy to avoid race conditions between threads.
final CameraDeviceWrapper cameraDeviceToClose = cameraDevice;
if (cameraDeviceToClose != null) {
cameraDeviceToClose.close();
cameraDevice = null;

// Closing the CameraDevice without closing the CameraCaptureSession is recommended
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_android/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Android implementation of the camera plugin.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22

version: 0.10.10+2
version: 0.10.10+3

environment:
sdk: ^3.6.0
Expand Down