Skip to content

Commit

Permalink
Add disconnection from service to free up references. (#91)
Browse files Browse the repository at this point in the history
  • Loading branch information
polina-c committed Jul 13, 2023
1 parent 9b97f84 commit 5675231
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 15 deletions.
1 change: 0 additions & 1 deletion doc/TROUBLESHOOT.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

4 changes: 4 additions & 0 deletions pkgs/leak_tracker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 7.0.8

* Disconnect from service after obtaining retaining paths.

# 7.0.7

* Protect from identityHashCode equal to 0.
Expand Down
6 changes: 5 additions & 1 deletion pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -187,14 +188,17 @@ 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 = 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) {
Expand Down
8 changes: 6 additions & 2 deletions pkgs/leak_tracker/lib/src/leak_tracking/orchestration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -95,8 +96,8 @@ Future<Leaks> 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();
}

Expand Down Expand Up @@ -173,10 +174,13 @@ Future<void> forceGC({
/// https://github.com/dart-lang/sdk/blob/3e80d29fd6fec56187d651ce22ea81f1e8732214/runtime/vm/object_graph.cc#L1803
Future<String?> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ Future<Connection> 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);

Expand All @@ -52,6 +50,8 @@ Future<Connection> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<RetainingPath?> obtainRetainingPath(Type type, int code) async {
assert(code > 0);

final connection = await connect();

Future<RetainingPath?> obtainRetainingPath(
Connection connection,
Type type,
int code,
) async {
final fp = _ObjectFingerprint(type, code);
final theObject = await _objectInIsolate(connection, fp);
if (theObject == null) return null;
Expand Down
2 changes: 1 addition & 1 deletion pkgs/leak_tracker/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>();
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...'),
Expand Down

0 comments on commit 5675231

Please sign in to comment.