diff --git a/.vscode/launch.json b/.vscode/launch.json index a8bfedc4..5ef6ce9e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -11,6 +11,12 @@ "type": "dart", "program": "main.dart", }, + { + "name": "pub_get", + "request": "launch", + "type": "node-terminal", + "command": "sh tool/pub_get.sh", + }, { "name": "minimal_flutter", "cwd": "examples/minimal_flutter", diff --git a/doc/TROUBLESHOOT.md b/doc/TROUBLESHOOT.md index b46bc0ad..e7eb008f 100644 --- a/doc/TROUBLESHOOT.md +++ b/doc/TROUBLESHOOT.md @@ -9,11 +9,11 @@ This page describes how to troubleshoot memory leaks. See other information on m If leak tracker detected a leak in your application or test, first check if the leak matches a [known simple case](#known-simple-cases), and, if no, switch to [more complicated troubleshooting](#more-complicated-cases). -## Known simple cases +## Check known simple cases ### 1. The test holds a disposed object -TODO: add steps. +TODO: add example and steps. ## Collect additional information @@ -44,7 +44,15 @@ For collecting debugging information in tests, temporarily pass an instance of ` ``` testWidgets('My test', (WidgetTester tester) async { ... - }, leakTrackingConfig: LeakTrackingTestConfig.debug()); + }, leakTrackingTestConfig: LeakTrackingTestConfig.debug()); +``` + +Or, you can temporarily set global flag, to make all tests collecting debug information: + +``` +setUpAll(() { + collectDebugInformationForLeaks = true; +}); ``` **Applications** @@ -56,6 +64,27 @@ For collecting debugging information in your running application, the options ar TODO: link DevTools documentation with explanation +## Verify object references + +If you expect an object to be not referenced at some point, +but not sure, you can validate it by temporaryly adding assertion. + +``` +final ref = WeakReference(myObject); +myObject = null; +await forceGC(); +if (ref.target == null) { + throw StateError('Validated that myObject is not held from garbage collection.'); +} else { + print(await formattedRetainingPath(ref)); + throw StateError('myObject is reachable from root. See console output for the retaining path.'); +} +``` + +IMPORTANT: this code will not work in release mode, so +you need to run it with flag `--debug` or `--profile`, or, +if it is test, by clicking `Debug` near your test name in IDE. + ## Known complicated cases ### 1. More than one closure context diff --git a/pkgs/leak_tracker/CHANGELOG.md b/pkgs/leak_tracker/CHANGELOG.md index f96d6305..aa4d7e8f 100644 --- a/pkgs/leak_tracker/CHANGELOG.md +++ b/pkgs/leak_tracker/CHANGELOG.md @@ -1,3 +1,8 @@ +# 7.0.6 + +* Add helpers for troubleshooting. +* Handle generic arguments for retaining path detection. + # 7.0.5 * Convert to multi-package. diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/_formatting.dart b/pkgs/leak_tracker/lib/src/leak_tracking/_formatting.dart index a40bcba1..efb6e4ca 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/_formatting.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/_formatting.dart @@ -11,7 +11,7 @@ import '../shared/_util.dart'; String contextToString(Object? object) { return switch (object) { StackTrace() => _formatStackTrace(object), - RetainingPath() => _retainingPathToString(object), + RetainingPath() => retainingPathToString(object), _ => object.toString(), }; } @@ -42,7 +42,7 @@ String removeLeakTrackingLines(String stackTrace) { return lines.join('\n'); } -String _retainingPathToString(RetainingPath retainingPath) { +String retainingPathToString(RetainingPath retainingPath) { final StringBuffer buffer = StringBuffer(); buffer.writeln( 'References that retain the object from garbage collection.', diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/_gc_counter.dart b/pkgs/leak_tracker/lib/src/leak_tracking/_gc_counter.dart index 6de94e0b..6f10eb55 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/_gc_counter.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/_gc_counter.dart @@ -10,7 +10,7 @@ class GcCounter { int get gcCount => reachabilityBarrier; } -/// Delta of GC time, enough for a non reachable object to be GCed. +/// Delta of GC cycles, enough for a non reachable object to be GCed. /// /// Theoretically, 2 should be enough, however it gives false positives /// if there is no activity in the application for ~5 minutes. diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/leak_tracker_model.dart b/pkgs/leak_tracker/lib/src/leak_tracking/leak_tracker_model.dart index 39069b5f..61564a50 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/leak_tracker_model.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/leak_tracker_model.dart @@ -4,6 +4,9 @@ import '../shared/shared_model.dart'; +/// If true, the leak tracker will collect debug information for leaks. +bool collectDebugInformationForLeaks = false; + /// Handler to collect leak summary. typedef LeakSummaryCallback = void Function(LeakSummary); diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart b/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart index 3bd73810..37783ba1 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart @@ -5,12 +5,12 @@ import 'dart:async'; import 'dart:developer'; -import 'package:clock/clock.dart'; - import '../shared/shared_model.dart'; +import '_formatting.dart'; import '_gc_counter.dart'; import 'leak_tracker.dart'; import 'leak_tracker_model.dart'; +import 'retaining_path/_retaining_path.dart'; /// Asynchronous callback. /// @@ -100,8 +100,8 @@ Future withLeakTracking( await checkNonGCed(); } - await _forceGC( - gcCycles: gcCountBuffer, + await forceGC( + fullGcCycles: gcCountBuffer, timeout: timeoutForFinalGarbageCollection, ); @@ -124,22 +124,57 @@ Future withLeakTracking( } /// Forces garbage collection by aggressive memory allocation. -Future _forceGC({required int gcCycles, Duration? timeout}) async { - final start = clock.now(); - final barrier = reachabilityBarrier; +/// +/// Verifies that garbage collection happened using [reachabilityBarrier]. +/// Does not work in web and in release mode. +/// +/// Use [timeout] to limit waiting time. +/// Use [fullGcCycles] to force multiple garbage collections. +/// +/// The method is helpful for testing in combination with [WeakReference] to ensure +/// an object is not held by another object from garbage collection. +/// +/// For code example see +/// https://github.com/dart-lang/leak_tracker/blob/main/doc/TROUBLESHOOT.md +Future forceGC({ + Duration? timeout, + int fullGcCycles = 1, +}) async { + final Stopwatch? stopwatch = timeout == null ? null : (Stopwatch()..start()); + final int barrier = reachabilityBarrier; - final storage = >[]; + final List> storage = >[]; void allocateMemory() { - storage.add(Iterable.generate(10000, (_) => DateTime.now()).toList()); - if (storage.length > 100) storage.removeAt(0); + storage.add( + Iterable.generate(10000, (_) => DateTime.now()).toList(), + ); + if (storage.length > 100) { + storage.removeAt(0); + } } - while (reachabilityBarrier < barrier + gcCycles) { - if (timeout != null && clock.now().difference(start) > timeout) { + while (reachabilityBarrier < barrier + fullGcCycles) { + if ((stopwatch?.elapsed ?? Duration.zero) > (timeout ?? Duration.zero)) { throw TimeoutException('forceGC timed out', timeout); } - await Future.delayed(const Duration()); + await Future.delayed(Duration.zero); allocateMemory(); } } + +/// Returns nicely formatted retaining path for the [ref.target]. +/// +/// If the object is garbage collected or not retained, returns null. +/// +/// Does not work in web and in release mode. +Future formattedRetainingPath(WeakReference ref) async { + if (ref.target == null) return null; + final path = await obtainRetainingPath( + ref.target.runtimeType, + identityHashCode(ref.target), + ); + + if (path == null) return null; + return retainingPathToString(path); +} diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_connection.dart b/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_connection.dart index eab35ce9..691a70ae 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_connection.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_connection.dart @@ -14,7 +14,7 @@ final _log = Logger('_connection.dart'); class Connection { Connection(this.service, this.isolates); - final List isolates; + final List isolates; final VmService service; } @@ -45,7 +45,7 @@ Future connect() async { throw error ?? Exception('Error connecting to service protocol'); }); await service.getVersion(); // Warming up and validating the connection. - final isolates = await _getIdForTwoIsolates(service); + final isolates = await _getTwoIsolates(service); final result = Connection(service, isolates); completer.complete(result); @@ -57,10 +57,10 @@ Future connect() async { /// Depending on environment (command line / IDE, Flutter / Dart), isolates may have different names, /// and there can be one or two. Sometimes the second one appears with latency. /// And sometimes there are two isolates with name 'main'. -Future> _getIdForTwoIsolates(VmService service) async { +Future> _getTwoIsolates(VmService service) async { _log.info('Started loading isolates...'); - final result = []; + final result = []; const isolatesToGet = 2; const watingTime = Duration(seconds: 2); @@ -69,7 +69,7 @@ Future> _getIdForTwoIsolates(VmService service) async { result.clear(); await _forEachIsolate( service, - (IsolateRef r) async => result.add(r.id!), + (IsolateRef r) async => result.add(r), ); if (result.length < isolatesToGet) { await Future.delayed(const Duration(milliseconds: 100)); diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_retaining_path.dart b/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_retaining_path.dart index d13bbb07..48777478 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_retaining_path.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/retaining_path/_retaining_path.dart @@ -17,7 +17,7 @@ Future obtainRetainingPath(Type type, int code) async { if (theObject == null) return null; final result = await connection.service.getRetainingPath( - theObject.isolateId, + theObject.isolateRef.id!, theObject.itemId, 100000, ); @@ -30,20 +30,30 @@ class _ObjectFingerprint { final Type type; final int code; + + String get typeNameWithoutArgs { + final name = type.toString(); + final index = name.indexOf('<'); + if (index == -1) return name; + return name.substring(0, index); + } } Future<_ItemInIsolate?> _objectInIsolate( Connection connection, _ObjectFingerprint object, ) async { - final classes = await _findClasses(connection, object.type.toString()); + final classes = await _findClasses(connection, object.typeNameWithoutArgs); for (final theClass in classes) { - const pathLengthLimit = 10000000; + // TODO(polina-c): remove when issue is fixed + // https://github.com/dart-lang/sdk/issues/52893 + if (theClass.name == 'TypeParameters') continue; + final instances = (await connection.service.getInstances( - theClass.isolateId, + theClass.isolateRef.id!, theClass.itemId, - pathLengthLimit, + 1000000000, )) .instances ?? []; @@ -53,7 +63,10 @@ Future<_ItemInIsolate?> _objectInIsolate( objRef is InstanceRef && objRef.identityHashCode == object.code, ); if (result != null) { - return _ItemInIsolate(isolateId: theClass.isolateId, itemId: result.id!); + return _ItemInIsolate( + isolateRef: theClass.isolateRef, + itemId: result.id!, + ); } } @@ -64,13 +77,16 @@ Future<_ItemInIsolate?> _objectInIsolate( /// /// It can be class or object. class _ItemInIsolate { - _ItemInIsolate({required this.isolateId, required this.itemId}); + _ItemInIsolate({required this.isolateRef, required this.itemId, this.name}); - /// Id of the isolate. - final String isolateId; + /// The isolate. + final IsolateRef isolateRef; /// Id of the item in the isolate. final String itemId; + + /// Name of the item, for debugging purposes. + final String? name; } Future> _findClasses( @@ -79,8 +95,8 @@ Future> _findClasses( ) async { final result = <_ItemInIsolate>[]; - for (final isolateId in connection.isolates) { - var classes = await connection.service.getClassList(isolateId); + for (final isolate in connection.isolates) { + var classes = await connection.service.getClassList(isolate.id!); const watingTime = Duration(seconds: 2); final stopwatch = Stopwatch()..start(); @@ -88,7 +104,7 @@ Future> _findClasses( // In the beginning list of classes may be empty. while (classes.classes?.isEmpty ?? true && stopwatch.elapsed < watingTime) { await Future.delayed(const Duration(milliseconds: 100)); - classes = await connection.service.getClassList(isolateId); + classes = await connection.service.getClassList(isolate.id!); } if (classes.classes?.isEmpty ?? true) { throw StateError('Could not get list of classes.'); @@ -96,10 +112,14 @@ Future> _findClasses( final filtered = classes.classes?.where((ref) => runtimeClassName == ref.name) ?? []; + result.addAll( filtered.map( - (classRef) => - _ItemInIsolate(itemId: classRef.id!, isolateId: isolateId), + (classRef) => _ItemInIsolate( + itemId: classRef.id!, + isolateRef: isolate, + name: classRef.name, + ), ), ); } diff --git a/pkgs/leak_tracker/pubspec.yaml b/pkgs/leak_tracker/pubspec.yaml index 94409a99..15f975f3 100644 --- a/pkgs/leak_tracker/pubspec.yaml +++ b/pkgs/leak_tracker/pubspec.yaml @@ -1,5 +1,5 @@ name: leak_tracker -version: 7.0.4 +version: 7.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 @@ -13,7 +13,7 @@ dependencies: logging: ^1.1.1 meta: ^1.8.0 path: ^1.8.3 - vm_service: '>=11.6.0 <13.0.0' + vm_service: '>=11.7.2 <13.0.0' web_socket_channel: ^2.1.0 dev_dependencies: diff --git a/pkgs/leak_tracker/test/debug/README.md b/pkgs/leak_tracker/test/debug/README.md index 0112ba57..94941847 100644 --- a/pkgs/leak_tracker/test/debug/README.md +++ b/pkgs/leak_tracker/test/debug/README.md @@ -4,3 +4,5 @@ To run them locally: ``` dart test --debug test/dart_debug ``` + +Or click 'Debug' near the test name in your IDE. diff --git a/pkgs/leak_tracker/test/debug/leak_tracking/orchestration_test.dart b/pkgs/leak_tracker/test/debug/leak_tracking/orchestration_test.dart new file mode 100644 index 00000000..4197d8fe --- /dev/null +++ b/pkgs/leak_tracker/test/debug/leak_tracking/orchestration_test.dart @@ -0,0 +1,21 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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 'package:leak_tracker/leak_tracker.dart'; +import 'package:test/test.dart'; + +class MyClass { + MyClass(); + + final Stopwatch stopwatch = Stopwatch(); + WeakReference get ref => WeakReference(stopwatch); +} + +void main() { + final myObject = MyClass(); + test('formattedRetainingPath returns path', () async { + final path = await formattedRetainingPath(myObject.ref); + expect(path, contains('_test.dart/MyClass:stopwatch')); + }); +} diff --git a/pkgs/leak_tracker/test/debug/leak_tracking/retaining_path/_retaining_path_test.dart b/pkgs/leak_tracker/test/debug/leak_tracking/retaining_path/_retaining_path_test.dart index d06b9c42..62a30e4d 100644 --- a/pkgs/leak_tracker/test/debug/leak_tracking/retaining_path/_retaining_path_test.dart +++ b/pkgs/leak_tracker/test/debug/leak_tracking/retaining_path/_retaining_path_test.dart @@ -13,6 +13,10 @@ class MyClass { MyClass(); } +class MyArgClass { + MyArgClass(); +} + final _logs = []; late StreamSubscription subscription; @@ -31,10 +35,22 @@ void main() { await subscription.cancel(); }); - test('$MyClass instance can be found.', () async { + test('Path for $MyClass instance is found.', () async { final instance = MyClass(); - final path = await obtainRetainingPath(MyClass, identityHashCode(instance)); + final path = await obtainRetainingPath( + instance.runtimeType, + identityHashCode(instance), + ); + expect(path!.elements, isNotEmpty); + }); + + test('Path for class with generic arg is found.', () async { + final instance = MyArgClass(); + final path = await obtainRetainingPath( + instance.runtimeType, + identityHashCode(instance), + ); expect(path!.elements, isNotEmpty); }); diff --git a/pkgs/leak_tracker/test/release/leak_tracking/orchestration_test.dart b/pkgs/leak_tracker/test/release/leak_tracking/orchestration_test.dart index 564e2cc4..78fe6aff 100644 --- a/pkgs/leak_tracker/test/release/leak_tracking/orchestration_test.dart +++ b/pkgs/leak_tracker/test/release/leak_tracking/orchestration_test.dart @@ -2,6 +2,8 @@ // 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:async'; + import 'package:leak_tracker/src/leak_tracking/orchestration.dart'; import 'package:test/test.dart'; @@ -19,4 +21,21 @@ void main() { await withLeakTracking(() async {}); }); + + test('forceGC forces gc', () async { + Object? myObject = [1, 2, 3, 4, 5]; + final ref = WeakReference(myObject); + myObject = null; + + await forceGC(); + + expect(ref.target, null); + }); + + test('forceGC times out', () async { + await expectLater( + forceGC(timeout: Duration.zero), + throwsA(isA()), + ); + }); } diff --git a/pkgs/leak_tracker_flutter_test/test/end_to_end_test.dart b/pkgs/leak_tracker_flutter_test/test/end_to_end_test.dart index 246e98ed..855fa623 100644 --- a/pkgs/leak_tracker_flutter_test/test/end_to_end_test.dart +++ b/pkgs/leak_tracker_flutter_test/test/end_to_end_test.dart @@ -27,7 +27,7 @@ void main() { (WidgetTester tester) async { await tester.pumpWidget(StatelessLeakingWidget()); }, - leakTrackingConfig: LeakTrackingTestConfig( + leakTrackingTestConfig: LeakTrackingTestConfig( onLeaks: (Leaks theLeaks) { leaks = theLeaks; }, @@ -36,7 +36,42 @@ void main() { ); tearDown( - () => _verifyLeaks(leaks, expectedNotDisposed: 1, expectedNotGCed: 1), + () => _verifyLeaks( + leaks, + expectedNotDisposed: 1, + expectedNotGCed: 1, + shouldContainDebugInfo: false, + ), + ); + }); + + group('Leak tracker respects flag collectDebugInformationForLeaks', () { + late Leaks leaks; + + setUp( + () => collectDebugInformationForLeaks = true, + ); + + testWidgetsWithLeakTracking( + 'for $StatelessLeakingWidget', + (WidgetTester tester) async { + await tester.pumpWidget(StatelessLeakingWidget()); + }, + leakTrackingTestConfig: LeakTrackingTestConfig( + onLeaks: (Leaks theLeaks) { + leaks = theLeaks; + }, + failTestOnLeaks: false, + ), + ); + + tearDown( + () => _verifyLeaks( + leaks, + expectedNotDisposed: 1, + expectedNotGCed: 1, + shouldContainDebugInfo: false, + ), ); }); @@ -48,7 +83,7 @@ void main() { (WidgetTester tester) async { await tester.pumpWidget(StatelessLeakingWidget()); }, - leakTrackingConfig: LeakTrackingTestConfig( + leakTrackingTestConfig: LeakTrackingTestConfig( onLeaks: (Leaks theLeaks) { leaks = theLeaks; }, @@ -92,6 +127,7 @@ void _verifyLeaks( Leaks leaks, { int expectedNotDisposed = 0, int expectedNotGCed = 0, + required bool shouldContainDebugInfo, }) { const String linkToLeakTracker = 'https://github.com/dart-lang/leak_tracker'; @@ -104,14 +140,31 @@ void _verifyLeaks( ), ); - _verifyLeakList(leaks.notDisposed, expectedNotDisposed); - _verifyLeakList(leaks.notGCed, expectedNotGCed); + _verifyLeakList( + leaks.notDisposed, + expectedNotDisposed, + shouldContainDebugInfo, + ); + _verifyLeakList( + leaks.notGCed, + expectedNotGCed, + shouldContainDebugInfo, + ); } -void _verifyLeakList(List list, int expectedCount) { +void _verifyLeakList( + List list, + int expectedCount, + bool shouldContainDebugInfo, +) { expect(list.length, expectedCount); for (final LeakReport leak in list) { + if (shouldContainDebugInfo) { + expect(leak.context, isNotEmpty); + } else { + expect(leak.context ?? {}, isEmpty); + } expect(leak.trackedClass, contains(LeakTrackedClass.library)); expect(leak.trackedClass, contains('$LeakTrackedClass')); } diff --git a/pkgs/leak_tracker_flutter_test/test/test_infra/helpers.dart b/pkgs/leak_tracker_flutter_test/test/test_infra/helpers.dart index 5a159c85..6905daef 100644 --- a/pkgs/leak_tracker_flutter_test/test/test_infra/helpers.dart +++ b/pkgs/leak_tracker_flutter_test/test/test_infra/helpers.dart @@ -32,13 +32,18 @@ void testWidgetsWithLeakTracking( bool semanticsEnabled = true, TestVariant variant = const DefaultTestVariant(), dynamic tags, - LeakTrackingTestConfig leakTrackingConfig = const LeakTrackingTestConfig(), + LeakTrackingTestConfig? leakTrackingTestConfig, }) { + final config = leakTrackingTestConfig ?? + (collectDebugInformationForLeaks + ? LeakTrackingTestConfig.debug() + : const LeakTrackingTestConfig()); + Future wrappedCallback(WidgetTester tester) async { await withFlutterLeakTracking( () async => callback(tester), tester, - leakTrackingConfig, + config, ); } diff --git a/tool/analyze.sh b/tool/analyze.sh index f4e26984..2a34db92 100644 --- a/tool/analyze.sh +++ b/tool/analyze.sh @@ -7,27 +7,24 @@ # Fast fail the script on failures. set -ex +sh ./tool/pub_get.sh + cd examples/autosnapshotting -flutter pub get flutter analyze --fatal-infos cd - cd examples/minimal_flutter -flutter pub get flutter analyze --fatal-infos cd - cd pkgs/leak_tracker -dart pub get dart analyze --fatal-infos cd - cd pkgs/leak_tracker_flutter_test -flutter pub get flutter analyze --fatal-infos cd - cd pkgs/leak_tracker_testing -flutter pub get dart analyze --fatal-infos cd - diff --git a/tool/pub_get.sh b/tool/pub_get.sh new file mode 100644 index 00000000..5df56ec2 --- /dev/null +++ b/tool/pub_get.sh @@ -0,0 +1,28 @@ +#!/bin/bash + +# Copyright 2023 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# Fast fail the script on failures. +set -ex + +cd examples/autosnapshotting +flutter pub get +cd - + +cd examples/minimal_flutter +flutter pub get +cd - + +cd pkgs/leak_tracker +dart pub get +cd - + +cd pkgs/leak_tracker_flutter_test +flutter pub get +cd - + +cd pkgs/leak_tracker_testing +flutter pub get +cd -