Fix and test cover case of customized gcCountBuffer

CONTRIBUTING.md

@@ -0,0 +1,11 @@
+# Contributing code
+
+We gladly accept contributions via GitHub pull requests!
+
+## How to enable logs
+
+To temporary enable logs, add this line to `main`:
+
+```
+Logger.root.onRecord.listen((LogRecord record) => print(record.message));
+```

README.md

@@ -18,6 +18,12 @@ Documentation:
 1. [Understand leak tracking concepts](doc/CONCEPTS.md)
 2. [Troubleshoot memory leaks](doc/TROUBLESHOOT.md)
 
+## Contributing and development
+
+Contributions welcome! See our
+[contributing page](https://github.com/dart-lang/leak_tracker/blob/main/CONTRIBUTING.md)
+for an overview of how to build and contribute to the project.
+
 ## Packages
 
 | Package | Description | Version |

pkgs/leak_tracker/CHANGELOG.md

@@ -1,3 +1,7 @@
+# 8.0.3
+
+* Fix and test cover case of customized `gcCountBuffer`.
+
 # 8.0.2
 
 * Improve performance.

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

@@ -222,6 +222,7 @@ class ObjectTracker implements LeakProvider {
 
   Future<void> _addRetainingPath(List<int> objectsToGetPath) async {
     final connection = await connect();
+
     final pathSetters = objectsToGetPath.map((code) async {
       final record = _objects.notGCed[code]!;
       final path =
@@ -230,8 +231,12 @@ class ObjectTracker implements LeakProvider {
         record.setContext(ContextKeys.retainingPath, path);
       }
     });
-    await Future.wait(pathSetters);
-    disconnect();
+
+    await Future.wait(
+      pathSetters,
+      eagerError: true,
+      cleanUp: (_) => disconnect(),
+    );
   }
 
   ObjectRecord _notGCed(int code) {

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

@@ -36,7 +36,9 @@ void enableLeakTracking({
   bool resetIfAlreadyEnabled = false,
 }) {
   assert(() {
-    final theConfig = config ??= const LeakTrackingConfiguration();
+    final theConfig = config ??= const LeakTrackingConfiguration(
+      gcCountBuffer: defaultGcCountBuffer,
+    );
     if (_objectTracker.value != null) {
       if (!resetIfAlreadyEnabled) {
         throw StateError('Leak tracking is already enabled.');

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

@@ -95,7 +95,7 @@ class LeakTrackingConfiguration {
     this.checkPeriod = const Duration(seconds: 1),
     this.disposalTimeBuffer = const Duration(milliseconds: 100),
     this.leakDiagnosticConfig = const LeakDiagnosticConfig(),
-    this.gcCountBuffer = defaultGcCountBuffer,
+    required this.gcCountBuffer,
   });
 
   /// The leak tracker:

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

@@ -5,13 +5,17 @@
 import 'dart:async';
 import 'dart:developer';
 
+import 'package:logging/logging.dart';
+
 import '../shared/shared_model.dart';
 import '_formatting.dart';
 import 'leak_tracker.dart';
 import 'leak_tracker_model.dart';
 import 'retaining_path/_connection.dart';
 import 'retaining_path/_retaining_path.dart';
 
+final _log = Logger('orchestration.dart');
+
 /// Asynchronous callback.
 ///
 /// The prefix `Dart` is used to avoid conflict with Flutter's [AsyncCallback].
@@ -80,12 +84,21 @@ Future<Leaks> withLeakTracking(
   AsyncCodeRunner? asyncCodeRunner,
   int gcCountBuffer = defaultGcCountBuffer,
 }) async {
+  if (gcCountBuffer <= 0) {
+    throw ArgumentError.value(
+      gcCountBuffer,
+      'gcCountBuffer',
+      'Must be positive.',
+    );
+  }
+
   if (callback == null) return Leaks({});
 
   enableLeakTracking(
     resetIfAlreadyEnabled: true,
     config: LeakTrackingConfiguration.passive(
       leakDiagnosticConfig: leakDiagnosticConfig,
+      gcCountBuffer: gcCountBuffer,
     ),
   );
 
@@ -108,9 +121,7 @@ Future<Leaks> withLeakTracking(
           fullGcCycles: gcCountBuffer,
           timeout: timeoutForFinalGarbageCollection,
         );
-
         leaks = await collectLeaks();
-
         if ((leaks?.total ?? 0) > 0 && shouldThrowOnLeaks) {
           // `expect` should not be used here, because, when the method is used
           // from Flutter, the packages `test` and `flutter_test` conflict.
@@ -144,6 +155,7 @@ Future<void> forceGC({
   Duration? timeout,
   int fullGcCycles = 1,
 }) async {
+  _log.info('Forcing garbage collection with fullGcCycles = $fullGcCycles...');
   final Stopwatch? stopwatch = timeout == null ? null : (Stopwatch()..start());
   final int barrier = reachabilityBarrier;
 
@@ -163,6 +175,7 @@ Future<void> forceGC({
     await Future<void>.delayed(Duration.zero);
     allocateMemory();
   }
+  _log.info('Done forcing garbage collection.');
 }
 
 /// Returns nicely formatted retaining path for the [ref.target].

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

@@ -20,7 +20,13 @@ class Connection {
 
 Completer<Connection>? _completer;
 
-void disconnect() => _completer = null;
+void disconnect() {
+  _completer?.completeError(
+    StateError('Disconnected from vm service protocol.'),
+  );
+  _completer = null;
+  _log.info('Disconnected from vm service protocol.');
+}
 
 Future<Connection> connect() async {
   if (_completer != null) {
@@ -35,10 +41,14 @@ Future<Connection> connect() async {
 
   final uri = info.serverWebSocketUri;
   if (uri == null) {
-    throw StateError(
-      'Leak troubleshooting is not available in release mode. Run your application or test with flag "--debug" '
-      '(Not supported for Flutter yet: https://github.com/flutter/flutter/issues/127331).',
+    _completer = null;
+    completer.completeError(
+      StateError(
+        'Leak troubleshooting is not available in release mode. Run your application or test with flag "--debug" '
+        '(Not supported for Flutter yet: https://github.com/flutter/flutter/issues/127331).',
+      ),
     );
+    return await completer.future;
   }
 
   final service = await _connectWithWebSocket(uri, _handleError);
@@ -47,10 +57,14 @@ Future<Connection> connect() async {
 
   final result = Connection(service, isolates);
   completer.complete(result);
+  _log.info('Connected to vm service protocol.');
   return result;
 }
 
-void _handleError(Object? error) => throw error ?? Exception('Unknown error');
+void _handleError(Object? error) {
+  _log.info('Error in vm service protocol: $error');
+  throw error ?? Exception('Unknown error');
+}
 
 /// Tries to wait for two isolates to be available.
 ///

pkgs/leak_tracker/pubspec.yaml

@@ -1,5 +1,5 @@
 name: leak_tracker
-version: 8.0.2
+version: 8.0.3
 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/debug/leak_tracking/end_to_end_test.dart

@@ -16,76 +16,82 @@ void main() {
 
   tearDown(() => disableLeakTracking());
 
-  test('Leak tracker respects maxRequestsForRetainingPath.', () async {
-    LeakTrackerGlobalSettings.maxRequestsForRetainingPath = 2;
-    final leaks = await withLeakTracking(
-      () async {
-        LeakingClass();
-        LeakingClass();
-        LeakingClass();
-      },
-      shouldThrowOnLeaks: false,
-      leakDiagnosticConfig: const LeakDiagnosticConfig(
-        collectRetainingPathForNonGCed: true,
-      ),
-    );
+  for (var gcCountBuffer in [1, defaultGcCountBuffer]) {
+    test('Leak tracker respects maxRequestsForRetainingPath, $gcCountBuffer.',
+        () async {
+      LeakTrackerGlobalSettings.maxRequestsForRetainingPath = 2;
+      final leaks = await withLeakTracking(
+        () async {
+          LeakingClass();
+          LeakingClass();
+          LeakingClass();
+        },
+        shouldThrowOnLeaks: false,
+        leakDiagnosticConfig: const LeakDiagnosticConfig(
+          collectRetainingPathForNonGCed: true,
+        ),
+        gcCountBuffer: gcCountBuffer,
+      );
 
-    const pathHeader = '  path: >';
+      const pathHeader = '  path: >';
 
-    expect(leaks.notGCed, hasLength(3));
-    expect(
-      () => expect(leaks, isLeakFree),
-      throwsA(
-        predicate(
-          (e) {
-            if (e is! TestFailure) {
-              throw 'Unexpected exception type: ${e.runtimeType}';
-            }
-            expect(pathHeader.allMatches(e.message!), hasLength(2));
-            return true;
-          },
+      expect(leaks.notGCed, hasLength(3));
+      expect(
+        () => expect(leaks, isLeakFree),
+        throwsA(
+          predicate(
+            (e) {
+              if (e is! TestFailure) {
+                throw 'Unexpected exception type: ${e.runtimeType}';
+              }
+              expect(pathHeader.allMatches(e.message!), hasLength(2));
+              return true;
+            },
+          ),
         ),
-      ),
-    );
-  });
+      );
+    });
 
-  test('Retaining path for not GCed object is reported.', () async {
-    final leaks = await withLeakTracking(
-      () async {
-        LeakingClass();
-      },
-      shouldThrowOnLeaks: false,
-      leakDiagnosticConfig: const LeakDiagnosticConfig(
-        collectRetainingPathForNonGCed: true,
-      ),
-    );
+    test('Retaining path for not GCed object is reported, $gcCountBuffer.',
+        () async {
+      final leaks = await withLeakTracking(
+        () async {
+          LeakingClass();
+        },
+        shouldThrowOnLeaks: false,
+        leakDiagnosticConfig: const LeakDiagnosticConfig(
+          collectRetainingPathForNonGCed: true,
+        ),
+        gcCountBuffer: gcCountBuffer,
+      );
 
-    const expectedRetainingPathTails = [
-      '/leak_tracker/test/test_infra/data/dart_classes.dart/_notGCedObjects',
-      'dart.core/_GrowableList:',
-      '/leak_tracker/test/test_infra/data/dart_classes.dart/LeakTrackedClass',
-    ];
+      const expectedRetainingPathTails = [
+        '/leak_tracker/test/test_infra/data/dart_classes.dart/_notGCedObjects',
+        'dart.core/_GrowableList:',
+        '/leak_tracker/test/test_infra/data/dart_classes.dart/LeakTrackedClass',
+      ];
 
-    expect(leaks.total, 2);
-    expect(
-      () => expect(leaks, isLeakFree),
-      throwsA(
-        predicate(
-          (e) {
-            if (e is! TestFailure) {
-              throw 'Unexpected exception type: ${e.runtimeType}';
-            }
-            _verifyRetainingPath(expectedRetainingPathTails, e.message!);
-            return true;
-          },
+      expect(leaks.total, 2);
+      expect(
+        () => expect(leaks, isLeakFree),
+        throwsA(
+          predicate(
+            (e) {
+              if (e is! TestFailure) {
+                throw 'Unexpected exception type: ${e.runtimeType}';
+              }
+              _verifyRetainingPath(expectedRetainingPathTails, e.message!);
+              return true;
+            },
+          ),
         ),
-      ),
-    );
+      );
 
-    final theLeak = leaks.notGCed.first;
-    expect(theLeak.trackedClass, contains(LeakTrackedClass.library));
-    expect(theLeak.trackedClass, contains('$LeakTrackedClass'));
-  });
+      final theLeak = leaks.notGCed.first;
+      expect(theLeak.trackedClass, contains(LeakTrackedClass.library));
+      expect(theLeak.trackedClass, contains('$LeakTrackedClass'));
+    });
+  }
 }
 
 void _verifyRetainingPath(

pkgs/leak_tracker/test/release/README.md

@@ -0,0 +1 @@
+Bots run tests in this folder with 'dart test', where asserts are on, but VM service is unavailable.

pkgs/leak_tracker/test/release/leak_tracking/_leak_checker_test.dart

@@ -17,7 +17,8 @@ class _SummaryValues {
     LeakType.notGCed: 3,
   });
 
-  static final nonZeroCopy = LeakSummary(<LeakType, int>{}..addAll(nonZero.totals));
+  static final nonZeroCopy =
+      LeakSummary(<LeakType, int>{}..addAll(nonZero.totals));
 }
 
 void main() {
@@ -70,7 +71,8 @@ void main() {
     );
 
     // Mock defaults match real configuration defaults.
-    const config = LeakTrackingConfiguration();
+    const config =
+        LeakTrackingConfiguration(gcCountBuffer: defaultGcCountBuffer);
     final checker = defaultLeakChecker();
     expect(config.notifyDevTools, checker.devToolsSink != null);
     expect(config.stdoutLeaks, checker.stdoutSink != null);