From 56752316847e0ddd023650d35a9c7a5da1581d96 Mon Sep 17 00:00:00 2001 From: Polina Cherkasova Date: Thu, 13 Jul 2023 10:27:20 -0700 Subject: [PATCH] Add disconnection from service to free up references. (#91) --- doc/TROUBLESHOOT.md | 1 - pkgs/leak_tracker/CHANGELOG.md | 4 ++++ .../lib/src/leak_tracking/_object_tracker.dart | 6 +++++- .../lib/src/leak_tracking/orchestration.dart | 8 ++++++-- .../leak_tracking/retaining_path/_connection.dart | 6 +++--- .../retaining_path/_retaining_path.dart | 10 +++++----- pkgs/leak_tracker/pubspec.yaml | 2 +- .../retaining_path/_retaining_path_test.dart | 13 +++++++++++-- 8 files changed, 35 insertions(+), 15 deletions(-) diff --git a/doc/TROUBLESHOOT.md b/doc/TROUBLESHOOT.md index 268a11cd..4ca2365b 100644 --- a/doc/TROUBLESHOOT.md +++ b/doc/TROUBLESHOOT.md @@ -97,4 +97,3 @@ TODO: add example Such cases are hard to troubleshoot. One way to fix them is to convert all closures, which reference the leaked type, to named methods. - diff --git a/pkgs/leak_tracker/CHANGELOG.md b/pkgs/leak_tracker/CHANGELOG.md index 429d66cf..eae6017a 100644 --- a/pkgs/leak_tracker/CHANGELOG.md +++ b/pkgs/leak_tracker/CHANGELOG.md @@ -1,3 +1,7 @@ +# 7.0.8 + +* Disconnect from service after obtaining retaining paths. + # 7.0.7 * Protect from identityHashCode equal to 0. diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart b/pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart index 7a2242f8..04ced641 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart @@ -10,6 +10,7 @@ import '_finalizer.dart'; import '_gc_counter.dart'; import '_object_record.dart'; import 'leak_tracker_model.dart'; +import 'retaining_path/_connection.dart'; import 'retaining_path/_retaining_path.dart'; /// Keeps collection of object records until @@ -187,14 +188,17 @@ class ObjectTracker implements LeakProvider { } Future _addRetainingPath(List objectsToGetPath) async { + final connection = await connect(); final pathSetters = objectsToGetPath.map((code) async { final record = _objects.notGCed[code]!; - final path = await obtainRetainingPath(record.type, record.code); + final path = + await obtainRetainingPath(connection, record.type, record.code); if (path != null) { record.setContext(ContextKeys.retainingPath, path); } }); await Future.wait(pathSetters); + disconnect(); } ObjectRecord _notGCed(int code) { diff --git a/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart b/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart index 2ee24017..b003d909 100644 --- a/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart +++ b/pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart @@ -10,6 +10,7 @@ import '_formatting.dart'; import '_gc_counter.dart'; import 'leak_tracker.dart'; import 'leak_tracker_model.dart'; +import 'retaining_path/_connection.dart'; import 'retaining_path/_retaining_path.dart'; /// Asynchronous callback. @@ -95,8 +96,8 @@ Future withLeakTracking( await asyncCodeRunner( () async { if (leakDiagnosticConfig.collectRetainingPathForNonGCed) { - // This early check is needed to collect retaing pathes before forced GC, - // because pathes are unavailable for GCed objects. + // This early check is needed to collect retaing paths before forced GC, + // because paths are unavailable for GCed objects. await checkNonGCed(); } @@ -173,10 +174,13 @@ Future forceGC({ /// https://github.com/dart-lang/sdk/blob/3e80d29fd6fec56187d651ce22ea81f1e8732214/runtime/vm/object_graph.cc#L1803 Future formattedRetainingPath(WeakReference ref) async { if (ref.target == null) return null; + final connection = await connect(); final path = await obtainRetainingPath( + connection, ref.target.runtimeType, identityHashCode(ref.target), ); + disconnect(); 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 691a70ae..5e55216b 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 @@ -41,9 +41,7 @@ Future connect() async { ); } - final service = await _connectWithWebSocket(uri, (error) { - throw error ?? Exception('Error connecting to service protocol'); - }); + final service = await _connectWithWebSocket(uri, _handleError); await service.getVersion(); // Warming up and validating the connection. final isolates = await _getTwoIsolates(service); @@ -52,6 +50,8 @@ Future connect() async { return result; } +void _handleError(Object? error) => throw error ?? Exception('Unknown error'); + /// Tries to wait for two isolates to be available. /// /// Depending on environment (command line / IDE, Flutter / Dart), isolates may have different names, 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 a649a7f5..610d3d2d 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 @@ -13,11 +13,11 @@ import '_connection.dart'; /// /// Does not work for objects that have [identityHashCode] equal to 0. /// https://github.com/dart-lang/sdk/blob/3e80d29fd6fec56187d651ce22ea81f1e8732214/runtime/vm/object_graph.cc#L1803 -Future obtainRetainingPath(Type type, int code) async { - assert(code > 0); - - final connection = await connect(); - +Future obtainRetainingPath( + Connection connection, + Type type, + int code, +) async { final fp = _ObjectFingerprint(type, code); final theObject = await _objectInIsolate(connection, fp); if (theObject == null) return null; diff --git a/pkgs/leak_tracker/pubspec.yaml b/pkgs/leak_tracker/pubspec.yaml index cd71ad5e..bf1f0165 100644 --- a/pkgs/leak_tracker/pubspec.yaml +++ b/pkgs/leak_tracker/pubspec.yaml @@ -1,5 +1,5 @@ name: leak_tracker -version: 7.0.7 +version: 7.0.8 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 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 62a30e4d..94533152 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 @@ -37,33 +37,42 @@ void main() { test('Path for $MyClass instance is found.', () async { final instance = MyClass(); + final connection = await connect(); final path = await obtainRetainingPath( + connection, instance.runtimeType, identityHashCode(instance), ); + disconnect(); expect(path!.elements, isNotEmpty); }); test('Path for class with generic arg is found.', () async { final instance = MyArgClass(); + final connection = await connect(); + final path = await obtainRetainingPath( + connection, instance.runtimeType, identityHashCode(instance), ); + disconnect(); expect(path!.elements, isNotEmpty); }); test('Connection is happening just once', () async { final instance1 = MyClass(); final instance2 = MyClass(); + final connection = await connect(); final obtainers = [ - obtainRetainingPath(MyClass, identityHashCode(instance1)), - obtainRetainingPath(MyClass, identityHashCode(instance2)), + obtainRetainingPath(connection, MyClass, identityHashCode(instance1)), + obtainRetainingPath(connection, MyClass, identityHashCode(instance2)), ]; await Future.wait(obtainers); + disconnect(); expect( _logs.where((item) => item == 'Connecting to vm service protocol...'),