Request retaining path more carefully. (#95)

pkgs/leak_tracker/CHANGELOG.md

@@ -1,3 +1,8 @@
+# 8.0.1
+
+* Handle SentinelException for retaining path.
+* Limit number of requests for retaining path.
+
 # 8.0.0
 
 * Enable turn on/off tracking for leak types.

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

@@ -2,7 +2,10 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'dart:math';
+
 import 'package:clock/clock.dart';
+import 'package:meta/meta.dart';
 
 import '../shared/_primitives.dart';
 import '../shared/shared_model.dart';
@@ -180,13 +183,36 @@ class ObjectTracker implements LeakProvider {
       }
     }
 
-    if (objectsToGetPath != null && objectsToGetPath.isNotEmpty) {
-      await _addRetainingPath(objectsToGetPath);
-    }
+    await processIfNeeded(
+      items: objectsToGetPath,
+      limit: LeakTrackerGlobalSettings.maxRequestsForRetainingPath,
+      processor: _addRetainingPath,
+    );
 
     _objects.assertIntegrity();
   }
 
+  /// Runs [processor] for first items from [items], at most [limit] items will be processed.
+  ///
+  /// Noop if [items] is null or empty.
+  /// Processes all items if [limit] is null.
+  @visibleForTesting
+  static Future<void> processIfNeeded<T>({
+    required List<T>? items,
+    required int? limit,
+    required Future<void> Function(List<T>) processor,
+  }) async {
+    if (items == null || items.isEmpty) return;
+
+    if (limit != null) {
+      items = items.sublist(
+        0,
+        min(limit, items.length),
+      );
+    }
+    await processor(items);
+  }
+
   Future<void> _addRetainingPath(List<int> objectsToGetPath) async {
     final connection = await connect();
     final pathSetters = objectsToGetPath.map((code) async {

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

@@ -13,6 +13,13 @@ class LeakTrackerGlobalSettings {
   /// If true, a warning will be printed when leak tracking is
   /// requested for a non-supported platform.
   static bool warnForNonSupportedPlatforms = true;
+
+  /// Limit for number of requests for retaining path per one round
+  /// of validation for leaks.
+  ///
+  /// If the number is too big, the performance may be seriously impacted.
+  /// If null, path will be requested without limit.
+  static int? maxRequestsForRetainingPath = 10;
 }
 
 /// Handler to collect leak summary.

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

@@ -22,13 +22,17 @@ Future<RetainingPath?> obtainRetainingPath(
   final theObject = await _objectInIsolate(connection, fp);
   if (theObject == null) return null;
 
-  final result = await connection.service.getRetainingPath(
-    theObject.isolateRef.id!,
-    theObject.itemId,
-    100000,
-  );
+  try {
+    final result = await connection.service.getRetainingPath(
+      theObject.isolateRef.id!,
+      theObject.itemId,
+      100000,
+    );
 
-  return result;
+    return result;
+  } on SentinelException {
+    return null;
+  }
 }
 
 class _ObjectFingerprint {

pkgs/leak_tracker/pubspec.yaml

@@ -1,5 +1,5 @@
 name: leak_tracker
-version: 8.0.0
+version: 8.0.1
 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

@@ -10,8 +10,45 @@ import '../../test_infra/data/dart_classes.dart';
 
 /// Tests for non-mocked public API of leak tracker.
 void main() {
+  setUp(() {
+    LeakTrackerGlobalSettings.maxRequestsForRetainingPath = null;
+  });
+
   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,
+      ),
+    );
+
+    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;
+          },
+        ),
+      ),
+    );
+  });
+
   test('Retaining path for not GCed object is reported.', () async {
     final leaks = await withLeakTracking(
       () async {
@@ -25,7 +62,7 @@ void main() {
 
     const expectedRetainingPathTails = [
       '/leak_tracker/test/test_infra/data/dart_classes.dart/_notGCedObjects',
-      'dart.core/_GrowableList:0',
+      'dart.core/_GrowableList:',
       '/leak_tracker/test/test_infra/data/dart_classes.dart/LeakTrackedClass',
     ];
 
@@ -52,18 +89,18 @@ void main() {
 }
 
 void _verifyRetainingPath(
-  List<String> expectedRetainingPathTails,
+  List<String> expectedRetainingPathFragments,
   String actualMessage,
 ) {
   int? previousIndex;
-  for (var item in expectedRetainingPathTails) {
-    final index = actualMessage.indexOf('$item\n');
+  for (var item in expectedRetainingPathFragments) {
+    final index = actualMessage.indexOf(item);
     if (previousIndex == null) {
       previousIndex = index;
       continue;
     }
 
-    expect(index > previousIndex, true);
+    expect(index, greaterThan(previousIndex));
     final stringBetweenItems = actualMessage.substring(previousIndex, index);
     expect(
       RegExp('^').allMatches(stringBetweenItems).length,

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

@@ -14,6 +14,64 @@ const String _trackedClass = 'trackedClass';
 const _disposalTimeBuffer = Duration(milliseconds: 100);
 
 void main() {
+  group('processIfNeeded', () {
+    for (var items in [null, <int>[]]) {
+      test('is noop for empty list, $items', () async {
+        int processorCalls = 0;
+
+        await ObjectTracker.processIfNeeded<int>(
+          items: items,
+          limit: 10,
+          processor: (List<int> items) async {
+            processorCalls++;
+          },
+        );
+
+        expect(processorCalls, 0);
+      });
+    }
+
+    for (var limit in [null, 100]) {
+      test('processes all for no limit or large limit, $limit', () async {
+        final itemsToProcess = [1, 2, 3];
+
+        int processorCalls = 0;
+        late final List<int> processedItems;
+
+        await ObjectTracker.processIfNeeded<int>(
+          items: itemsToProcess,
+          limit: limit,
+          processor: (List<int> items) async {
+            processorCalls++;
+            processedItems = items;
+          },
+        );
+
+        expect(processorCalls, 1);
+        expect(processedItems, itemsToProcess);
+      });
+    }
+
+    test('cuts for limit', () async {
+      final itemsToProcess = [1, 2, 3];
+
+      int processorCalls = 0;
+      late final List<int> processedItems;
+
+      await ObjectTracker.processIfNeeded<int>(
+        items: itemsToProcess,
+        limit: 2,
+        processor: (List<int> items) async {
+          processorCalls++;
+          processedItems = items;
+        },
+      );
+
+      expect(processorCalls, 1);
+      expect(processedItems, [1, 2]);
+    });
+  });
+
   group('$ObjectTracker handles duplicates', () {
     late ObjectTracker tracker;
     IdentityHashCode mockCoder(Object object) => 1;