Skip to content
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
3 changes: 2 additions & 1 deletion packages/camera/camera/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 0.12.1

* Fixes a crash where a `CameraController` could update its value after being disposed, throwing "A CameraController was used after being disposed." (see [flutter/flutter#184959](https://github.com/flutter/flutter/issues/184959)).
* Updates minimum supported SDK version to Flutter 3.38/Dart 3.10.

## 0.12.0+1
Expand Down
41 changes: 20 additions & 21 deletions packages/camera/camera/lib/src/camera_controller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,9 @@ class CameraController extends ValueNotifier<CameraValue> {
_deviceOrientationSubscription ??= CameraPlatform.instance
.onDeviceOrientationChanged()
.listen((DeviceOrientationChangedEvent event) {
value = value.copyWith(deviceOrientation: event.orientation);
if (!_isDisposed) {
value = value.copyWith(deviceOrientation: event.orientation);
}
});

_cameraId = await CameraPlatform.instance.createCameraWithSettings(
Expand All @@ -355,7 +357,9 @@ class CameraController extends ValueNotifier<CameraValue> {

unawaited(
CameraPlatform.instance.onCameraError(_cameraId).first.then((CameraErrorEvent event) {
value = value.copyWith(errorDescription: event.description);
if (!_isDisposed) {
value = value.copyWith(errorDescription: event.description);
}
}),
);

Expand All @@ -364,25 +368,20 @@ class CameraController extends ValueNotifier<CameraValue> {
imageFormatGroup: imageFormatGroup ?? ImageFormatGroup.unknown,
);

value = value.copyWith(
isInitialized: true,
description: description,
previewSize: await initializeCompleter.future.then(
(CameraInitializedEvent event) => Size(event.previewWidth, event.previewHeight),
),
exposureMode: await initializeCompleter.future.then(
(CameraInitializedEvent event) => event.exposureMode,
),
focusMode: await initializeCompleter.future.then(
(CameraInitializedEvent event) => event.focusMode,
),
exposurePointSupported: await initializeCompleter.future.then(
(CameraInitializedEvent event) => event.exposurePointSupported,
),
focusPointSupported: await initializeCompleter.future.then(
(CameraInitializedEvent event) => event.focusPointSupported,
),
);
final CameraInitializedEvent event = await initializeCompleter.future;

// The controller may be disposed while awaiting initialization above.
if (!_isDisposed) {
value = value.copyWith(
isInitialized: true,
description: description,
previewSize: Size(event.previewWidth, event.previewHeight),
exposureMode: event.exposureMode,
focusMode: event.focusMode,
exposurePointSupported: event.exposurePointSupported,
focusPointSupported: event.focusPointSupported,
);
}
} on PlatformException catch (e) {
throw CameraException(e.code, e.message);
Comment on lines 385 to 386

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If _initializeWithDescription fails with a PlatformException, the _deviceOrientationSubscription that was created at the start of the method is left active and is never cancelled. This leads to a resource leak, and also prevents the subscription from being recreated if initialize() is retried (since _deviceOrientationSubscription remains non-null).

We should cancel the subscription and set it to null in the catch block to ensure proper cleanup and allow successful retries.

Suggested change
} on PlatformException catch (e) {
throw CameraException(e.code, e.message);
} on PlatformException catch (e) {
unawaited(_deviceOrientationSubscription?.cancel());
_deviceOrientationSubscription = null;
throw CameraException(e.code, e.message);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

/gemini

This is pre-existing behavior unrelated to the bug this PR fixes (a CameraController updating its value after being disposed).

The subscription is already cancelled in dispose(). It only lingers if the controller is never disposed, which is independent of whether initialization succeeds or fails. And since onDeviceOrientationChanged() is a global stream, reusing it on retry is intentional rather than a leak.

To keep this PR focused, I'd prefer to leave this out of scope, but I'm happy to address it in a separate PR if a maintainer thinks it's worth changing.

} finally {
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description: A Flutter plugin for controlling the camera. Supports previewing
Dart.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.12.0+1
version: 0.12.1

environment:
sdk: ^3.10.0
Expand Down
71 changes: 71 additions & 0 deletions packages/camera/camera/test/camera_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3428,6 +3428,54 @@ void main() {
expect(cameraController.value.errorDescription, mockOnCameraErrorEvent.description);
});
});

group('does not update value after dispose', () {
test('when initialization completes after dispose', () async {
final platform = MockDelayedInitializeCameraPlatform();
CameraPlatform.instance = platform;

final cameraController = CameraController(
const CameraDescription(
name: 'cam',
lensDirection: CameraLensDirection.back,
sensorOrientation: 90,
),
ResolutionPreset.max,
);

final Future<void> initializeFuture = cameraController.initialize();
await pumpEventQueue();

// Dispose while initialization is still in flight, then let it complete.
final Future<void> disposeFuture = cameraController.dispose();
platform.initializeCameraCompleter.complete();

await expectLater(initializeFuture, completes);
await disposeFuture;
});

test('when a camera error event arrives after dispose', () async {
final platform = MockControllableErrorCameraPlatform();
CameraPlatform.instance = platform;

final cameraController = CameraController(
const CameraDescription(
name: 'cam',
lensDirection: CameraLensDirection.back,
sensorOrientation: 90,
),
ResolutionPreset.max,
);
await cameraController.initialize();
await cameraController.dispose();

// A camera error arriving after dispose must not update `value`.
platform.cameraErrorController.add(const CameraErrorEvent(13, 'late error'));
await pumpEventQueue();

expect(cameraController.value.errorDescription, isNull);
});
});
}

class MockCameraPlatform extends Mock with MockPlatformInterfaceMixin implements CameraPlatform {
Expand Down Expand Up @@ -3611,3 +3659,26 @@ class MockCameraDescription extends CameraDescription {
@override
String get name => 'back';
}

/// A platform whose [initializeCamera] only completes once
/// [initializeCameraCompleter] is completed, so a test can dispose the
/// controller while initialization is still in flight.
class MockDelayedInitializeCameraPlatform extends MockCameraPlatform {
final Completer<void> initializeCameraCompleter = Completer<void>();

@override
Future<void> initializeCamera(
int? cameraId, {
ImageFormatGroup? imageFormatGroup = ImageFormatGroup.unknown,
}) => initializeCameraCompleter.future;
}

/// A platform whose camera error events are driven by [cameraErrorController],
/// so a test can emit an error after the controller has been disposed.
class MockControllableErrorCameraPlatform extends MockCameraPlatform {
final StreamController<CameraErrorEvent> cameraErrorController =
StreamController<CameraErrorEvent>.broadcast();

@override
Stream<CameraErrorEvent> onCameraError(int cameraId) => cameraErrorController.stream;
}