Fix handling of double tracking for an object. (#226)

pkgs/leak_tracker/CHANGELOG.md

@@ -1,3 +1,7 @@
+# 10.0.6
+
+* Handle double tracking.
+
 # 10.0.5
 
 * Stop failing if finalization happened twice.

pkgs/leak_tracker/lib/src/leak_tracking/_object_record.dart

@@ -48,10 +48,10 @@ class ObjectRecord {
   int? _disposalGcCount;
 
   void setDisposed(int gcTime, DateTime time) {
-    // TODO(polina-c): handle double disposal in a better way
-    // https://github.com/dart-lang/leak_tracker/issues/118
-    // Noop if object is already disposed.
-    if (_disposalGcCount != null) return;
+    if (_disposalGcCount != null) {
+      // It is not responsibility of leak tracker to check for double disposal.
+      return;
+    }
     if (_gcedGcCount != null) {
       throw Exception(
           'The object $code should not be disposed after being GCed');
@@ -63,11 +63,9 @@ class ObjectRecord {
   DateTime? _gcedTime;
   int? _gcedGcCount;
   void setGCed(int gcCount, DateTime time) {
-    // TODO: throw exception if object is already GCed after fix of https://github.com/dart-lang/sdk/issues/55330
-    // Normally it should not happen, but sometimes finalizer is called twice.
-    // To repro, update next line to throw exception and run flutter tests with
-    // the updated leak_tracker.
-    if (_gcedGcCount != null) return;
+    if (_gcedGcCount != null) {
+      throw Exception('$trackedClass, $code is GCed twice');
+    }
     _gcedGcCount = gcCount;
     _gcedTime = time;
   }

pkgs/leak_tracker/lib/src/leak_tracking/_object_record_set.dart

@@ -12,7 +12,7 @@ import 'primitives/model.dart';
 
 @visibleForTesting
 class ObjectRecordSet {
-  ObjectRecordSet({this.coder = standardIdentityHashCoder});
+  ObjectRecordSet({@visibleForTesting this.coder = standardIdentityHashCoder});
 
   final IdentityHashCoder coder;
 
@@ -49,7 +49,7 @@ class ObjectRecordSet {
     if (list.isEmpty) _records.remove(record.code);
   }
 
-  ObjectRecord putIfAbsent(
+  ({ObjectRecord record, bool wasAbsent}) putIfAbsent(
     Object object,
     Map<String, dynamic>? context,
     PhaseSettings phase,
@@ -59,8 +59,9 @@ class ObjectRecordSet {
 
     final list = _records.putIfAbsent(code, () => []);
 
-    final existing = list.firstWhereOrNull((r) => r.ref.target == object);
-    if (existing != null) return existing;
+    final existing =
+        list.firstWhereOrNull((r) => identical(r.ref.target, object));
+    if (existing != null) return (record: existing, wasAbsent: false);
 
     final creationChecker = phase.ignoredLeaks.createdByTestHelpers
         ? CreationChecker(
@@ -78,7 +79,7 @@ class ObjectRecordSet {
 
     list.add(result);
     _length++;
-    return result;
+    return (record: result, wasAbsent: true);
   }
 
   int _length = 0;

pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart

@@ -62,9 +62,11 @@ class ObjectTracker implements LeakProvider {
     throwIfDisposed();
     if (phase.ignoreLeaks) return;
 
-    final record =
+    final (:record, :wasAbsent) =
         _objects.notGCed.putIfAbsent(object, context, phase, trackedClass);
 
+    if (!wasAbsent) return;
+
     if (phase.leakDiagnosticConfig.collectStackTraceOnStart) {
       record.setContext(ContextKeys.startCallstack, StackTrace.current);
     }

pkgs/leak_tracker/lib/src/leak_tracking/leak_tracking.dart

@@ -114,6 +114,8 @@ abstract class LeakTracking {
   /// Use [context] to provide additional information,
   /// that may help in leak troubleshooting.
   /// The value must be serializable.
+  ///
+  /// Noop if object creation is already dispatched.
   static void dispatchObjectCreated({
     required String library,
     required String className,
@@ -138,6 +140,8 @@ abstract class LeakTracking {
   /// Dispatches object disposal to the leak_tracker.
   ///
   /// See [dispatchObjectCreated] for parameters documentation.
+  ///
+  /// Noop if object disposal is already dispatched.
   static void dispatchObjectDisposed({
     required Object object,
     Map<String, dynamic>? context,

pkgs/leak_tracker/pubspec.yaml

@@ -1,5 +1,5 @@
 name: leak_tracker
-version: 10.0.5
+version: 10.0.6
 description: A framework for memory leak tracking for Dart and Flutter applications.
 repository: https://github.com/dart-lang/leak_tracker/tree/main/pkgs/leak_tracker
 

pkgs/leak_tracker/test/tests/leak_tracking/_object_record_set_test.dart

@@ -46,21 +46,28 @@ void main() {
 ObjectRecord _addItemAndValidate(ObjectRecordSet theSet, Object item) {
   final length = theSet.length;
 
-  final record = theSet.putIfAbsent(item, {}, _phase, '');
+  // Add first time.
+  final (record: record1, wasAbsent: wasAbsent1) =
+      theSet.putIfAbsent(item, {}, _phase, '');
   expect(theSet.length, length + 1);
-  expect(theSet.contains(record), true);
+  expect(theSet.contains(record1), true);
   expect(theSet.contains(_record), false);
+  expect(wasAbsent1, true);
 
-  expect(theSet.putIfAbsent(item, {}, _phase, ''), record);
+  // Add second time.
+  final (record: record2, wasAbsent: wasAbsent2) =
+      theSet.putIfAbsent(item, {}, _phase, '');
+  expect(identical(record1, record2), true);
   expect(theSet.length, length + 1);
+  expect(wasAbsent2, false);
 
   var count = 0;
   theSet.forEach((record) => count++);
   expect(count, theSet.length);
 
-  expect(theSet.record(item), record);
+  expect(theSet.record(item), record1);
 
-  return record;
+  return record1;
 }
 
 void _removeItemAndValidate(ObjectRecordSet theSet, ObjectRecord record) {

pkgs/leak_tracker/test/tests/leak_tracking/_object_tracker_test.dart

@@ -13,7 +13,7 @@ import 'package:test/test.dart';
 
 import '../../test_infra/data/dart_classes.dart';
 
-const String _trackedClass = 'trackedClass';
+const String _trackedClass = 'MyClass';
 const _disposalTime = Duration(milliseconds: 100);
 
 void main() {
@@ -86,23 +86,32 @@ void main() {
       );
     });
 
-    test('without failures.', () {
-      final object1 = [1, 2, 3];
-      final object2 = ['-'];
+    test('without failures', () async {
+      Object? object = [1, 2, 3];
+      final ref = WeakReference(object);
 
       tracker.startTracking(
-        object1,
+        object,
         context: null,
         trackedClass: _trackedClass,
         phase: const PhaseSettings(),
       );
 
       tracker.startTracking(
-        object2,
+        object,
         context: null,
         trackedClass: _trackedClass,
         phase: const PhaseSettings(),
       );
+
+      tracker.dispatchDisposal(object, context: null);
+      tracker.dispatchDisposal(object, context: null);
+
+      object = null;
+      await forceGC();
+      expect(ref.target, null);
+      // Pause to allow finalizers to run.
+      await Future<void>.delayed(const Duration(milliseconds: 4));
     });
   });
 
@@ -187,7 +196,7 @@ void main() {
       tracker.startTracking(
         theObject,
         context: null,
-        trackedClass: 'trackedClass',
+        trackedClass: _trackedClass,
         phase: const PhaseSettings(),
       );
       finalizerBuilder.gc(theObject);
@@ -206,7 +215,7 @@ void main() {
         tracker.startTracking(
           theObject,
           context: null,
-          trackedClass: 'trackedClass',
+          trackedClass: _trackedClass,
           phase: const PhaseSettings(),
         );
         tracker.dispatchDisposal(theObject, context: null);
@@ -232,7 +241,7 @@ void main() {
         tracker.startTracking(
           theObject,
           context: null,
-          trackedClass: 'trackedClass',
+          trackedClass: _trackedClass,
           phase: const PhaseSettings(),
         );
         tracker.dispatchDisposal(theObject, context: null);
@@ -355,7 +364,7 @@ void main() {
       time = time.add(_disposalTime);
       gcCounter.gcCount = gcCounter.gcCount + defaultNumberOfGcCycles;
 
-      // GC and verify leak contains callstacks.
+      // GC and verify leak contains callstack.
       await withClock(Clock.fixed(time), () async {
         finalizerBuilder.gc(theObject);
         final theLeak =

pkgs/leak_tracker_flutter_testing/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 3.0.6
+
+* Upgrade leak_tracker to 10.0.6.
+
 ## 3.0.5
 
 * Upgrade leak_tracker to 10.0.5.

pkgs/leak_tracker_flutter_testing/pubspec.yaml

@@ -1,5 +1,5 @@
 name: leak_tracker_flutter_testing
-version: 3.0.5
+version: 3.0.6
 description: An internal package to test leak tracking with Flutter.
 repository: https://github.com/dart-lang/leak_tracker/tree/main/pkgs/leak_tracker_flutter_testing
 
@@ -10,7 +10,7 @@ environment:
 dependencies:
   flutter:
     sdk: flutter
-  leak_tracker: '>=10.0.5 <11.0.0'
+  leak_tracker: '>=10.0.6 <11.0.0'
   leak_tracker_testing: '>=3.0.1 <4.0.0'
   matcher: ^0.12.16
   meta: ^1.8.0