Skip to content

[camera_android_camerax] Fixes premature garbage collection of native objects when app is under memory pressure #9287

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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_camerax/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.16+1

* Fixes premature garbage collection of native objects when app is under memory pressure.

## 0.6.16

* Fixes incorrect camera preview rotation for landscape-oriented devices.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ android {
lintOptions {
checkAllWarnings true
warningsAsErrors true
disable 'AndroidGradlePluginVersion', 'GradleDependency', 'InvalidPackage', 'NewerVersionAvailable'
disable 'AndroidGradlePluginVersion', 'GradleDependency', 'InvalidPackage', 'NewerVersionAvailable', 'UnsafeOptInUsageError'
Copy link
Contributor Author

@bparrishMines bparrishMines May 19, 2025

Choose a reason for hiding this comment

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

In #8618, I manually added this to the generated pigeon file CameraXLibrary.g.kt. To prevent the need to manually change generated code, I moved the disable here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Working on another camera PR (#9264) and I created a baseline lint file for that same UnsafeOptInUsageError lint. I think that might be better just in case the lint is valid for non-generated files, but let me know what you think!

If you agree and these other lints are from CameraXLibrary.g.kt, maybe we should remove them and just regenerate the baseline file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll wait for #9264 to land and then regen the baseline file for this PR

}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v25.3.1), do not edit directly.
// Autogenerated from Pigeon (v25.3.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon
@file:Suppress("UNCHECKED_CAST", "ArrayInDataClass", "UnsafeOptInUsageError")
@file:Suppress("UNCHECKED_CAST", "ArrayInDataClass")

package io.flutter.plugins.camerax

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Autogenerated from Pigeon (v25.3.1), do not edit directly.
// Autogenerated from Pigeon (v25.3.2), do not edit directly.
// See also: https://pub.dev/packages/pigeon
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers

Expand Down Expand Up @@ -252,8 +252,17 @@ class PigeonInstanceManager {
///
/// Returns the randomly generated id of the [instance] added.
int addDartCreatedInstance(PigeonInternalProxyApiBaseClass instance) {
assert(getIdentifier(instance) == null);

final int identifier = _nextUniqueIdentifier();
_addInstanceWithIdentifier(instance, identifier);
_identifiers[instance] = identifier;
_weakInstances[identifier] =
WeakReference<PigeonInternalProxyApiBaseClass>(instance);
_finalizer.attach(instance, identifier, detach: instance);

final PigeonInternalProxyApiBaseClass copy = instance.pigeon_copy();
_identifiers[copy] = identifier;
_strongInstances[identifier] = copy;
return identifier;
}

Expand Down Expand Up @@ -285,9 +294,15 @@ class PigeonInstanceManager {
/// it was removed. Returns `null` if [identifier] was not associated with
/// any strong reference.
///
/// This does not remove the weak referenced instance associated with
/// [identifier]. This can be done with [removeWeakReference].
/// Throws an `AssertionError` if the weak referenced instance associated with
/// [identifier] is not removed first. This can be done with
/// [removeWeakReference].
T? remove<T extends PigeonInternalProxyApiBaseClass>(int identifier) {
final T? instance = _weakInstances[identifier]?.target as T?;
assert(
instance == null,
'A strong instance with identifier $identifier is being removed despite the weak reference still existing: $instance',
);
return _strongInstances.remove(identifier) as T?;
}

Expand Down Expand Up @@ -338,27 +353,14 @@ class PigeonInstanceManager {
///
/// Throws assertion error if the instance or its identifier has already been
/// added.
///
/// Returns unique identifier of the [instance] added.
void addHostCreatedInstance(
PigeonInternalProxyApiBaseClass instance, int identifier) {
_addInstanceWithIdentifier(instance, identifier);
}

void _addInstanceWithIdentifier(
PigeonInternalProxyApiBaseClass instance, int identifier) {
assert(!containsIdentifier(identifier));
assert(getIdentifier(instance) == null);
assert(identifier >= 0);

_identifiers[instance] = identifier;
_weakInstances[identifier] =
WeakReference<PigeonInternalProxyApiBaseClass>(instance);
_finalizer.attach(instance, identifier, detach: instance);

final PigeonInternalProxyApiBaseClass copy = instance.pigeon_copy();
_identifiers[copy] = identifier;
_strongInstances[identifier] = copy;
_strongInstances[identifier] = instance;
}

/// Whether this manager contains the given [identifier].
Expand Down
4 changes: 2 additions & 2 deletions packages/camera/camera_android_camerax/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: camera_android_camerax
description: Android implementation of the camera plugin using the CameraX library.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_android_camerax
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.6.16
version: 0.6.16+1

environment:
sdk: ^3.6.0
Expand Down Expand Up @@ -31,7 +31,7 @@ dev_dependencies:
sdk: flutter
leak_tracker_flutter_testing: any
mockito: ^5.4.4
pigeon: ^25.3.1
pigeon: ^25.3.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify: this is a guess at the fix for flutter/flutter#152763 or have you determined this is the cause?

Copy link
Contributor Author

@bparrishMines bparrishMines May 21, 2025

Choose a reason for hiding this comment

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

It is a guess, but a very educated guess.

I haven't been able to reproduce the exact error from the camerax_issue, but I have been able to reproduce the issues from flutter/flutter#168531 and flutter/flutter#168306 on iOS. Both of these are similar to the camerax_issue because they are NullPointerExceptions caused by an instance that originates from the native side (e.g. ImageProxy). Both of these were solved by bumping pigeon to 25.3.2.

I think it is harder to reproduce the error on Android because garbage collection for Java is nondeterministic and it requires the ImageProxy to be garbage collected within a small time frame. But I did use a few debug logs to verify that the scenario described here does occur within the plugin. And that this change most likely prevents it. Although the scenario is pretty rare, so its not impossible that I was just lucky.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah gotcha makes sense! SGTM :)


topics:
- camera