Skip to content

Commit

Permalink
[stable] [vm] Fix leakage of file system watcher related data structures
Browse files Browse the repository at this point in the history
Closes #52703

TEST=runtime/tests/vm/dart/regress_52703_test.dart

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/309862
Bug: #52703
Change-Id: Idd4b153a9c7b19995f6c6c8bda99b878a21425fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311521
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
  • Loading branch information
srawlins authored and Commit Queue committed Jul 10, 2023
1 parent 7475c2c commit 9024e2b
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ This is a patch release that:
- Fixes a flow in flow analysis that causes it to sometimes ignore destructuring
assignments (issue [#52767]).
- Fixes a memory leak in Dart analyzer's file-watching (issue [#52791]).
- Fixes a memory leak of file system watcher related data structures (issue [#52793]).

[#52767]: https://github.com/dart-lang/sdk/issues/52767
[#52791]: https://github.com/dart-lang/sdk/issues/52791
[#52793]: https://github.com/dart-lang/sdk/issues/52793

## 3.0.5 - 2023-06-14

Expand Down
60 changes: 60 additions & 0 deletions runtime/tests/vm/dart/regress_52703_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// 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 'dart:async';
import 'dart:developer';
import 'dart:io';

import 'package:expect/expect.dart';
import 'package:path/path.dart' as path;
import 'package:vm_service/vm_service.dart';

import 'use_flag_test_helper.dart';
import '../../../tools/heapsnapshot/lib/src/analysis.dart';

void main() async {
if (buildDir.contains('Product')) return;

final dir = Directory('sdk');

StreamSubscription? current;
Future<void> iterate() async {
final temp = dir.watch().listen((event) {});
await current?.cancel();
current = dir.watch().listen((event) {});
await temp.cancel();
}

Future<void> finish() async {
await current?.cancel();
}

await withTempDir('ama-test', (String tempDir) async {
await iterate();

final before = countInstances(tempDir, '_BroadcastSubscription');
print('Before = $before');
for (int i = 0; i < 100; i++) {
await iterate();
}
final after = countInstances(tempDir, '_BroadcastSubscription');
print('After = $after');

await finish();

Expect.equals(0, (after - before));
});
}

int countInstances(String tempDir, String className) {
final snapshotFile = path.join(tempDir, 'foo.heapsnapshot');
NativeRuntime.writeHeapSnapshotToFile(snapshotFile);
final bytes = File(snapshotFile).readAsBytesSync();
File(snapshotFile).deleteSync();
final graph = HeapSnapshotGraph.fromChunks(
[bytes.buffer.asByteData(bytes.offsetInBytes, bytes.length)]);
final analysis = Analysis(graph);
return analysis
.filterByClassPatterns(analysis.reachableObjects, [className]).length;
}
26 changes: 18 additions & 8 deletions sdk/lib/_internal/vm/bin/file_patch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ abstract class _FileSystemWatcher {
final StreamController<FileSystemEvent> _broadcastController =
new StreamController<FileSystemEvent>.broadcast();

/// Subscription on the stream returned by [_watchPath].
///
/// Stored while piping events from that stream into [_broadcastController],
/// so it can be cancelled when [_broadcastController] is cancelled.
StreamSubscription? _sourceSubscription;

@patch
static Stream<FileSystemEvent> _watch(
String path, int events, bool recursive) {
Expand Down Expand Up @@ -193,7 +199,9 @@ abstract class _FileSystemWatcher {
}
_watcherPath = _idMap[pathId];
_watcherPath!.count++;
_pathWatched().pipe(_broadcastController);
_sourceSubscription = _pathWatched().listen(_broadcastController.add,
onError: _broadcastController.addError,
onDone: _broadcastController.close);
}

void _cancel() {
Expand Down Expand Up @@ -222,14 +230,16 @@ abstract class _FileSystemWatcher {
_doneWatcher();
_id = null;
}
_sourceSubscription?.cancel();
_sourceSubscription = null;
}

// Called when (and after) a new watcher instance is created and available.
void _newWatcher() {}
// Called when a watcher is no longer needed.
void _doneWatcher() {}
// Called when a new path is being watched.
Stream _pathWatched();
Stream<FileSystemEvent> _pathWatched();
// Called when a path is no longer being watched.
void _donePathWatched() {}

Expand Down Expand Up @@ -387,7 +397,7 @@ abstract class _FileSystemWatcher {
}

class _InotifyFileSystemWatcher extends _FileSystemWatcher {
static final Map<int, StreamController> _idMap = {};
static final Map<int, StreamController<FileSystemEvent>> _idMap = {};
static late StreamSubscription _subscription;

_InotifyFileSystemWatcher(path, events, recursive)
Expand All @@ -411,7 +421,7 @@ class _InotifyFileSystemWatcher extends _FileSystemWatcher {
_subscription.cancel();
}

Stream _pathWatched() {
Stream<FileSystemEvent> _pathWatched() {
var pathId = _watcherPath!.pathId;
if (!_idMap.containsKey(pathId)) {
_idMap[pathId] = new StreamController<FileSystemEvent>.broadcast();
Expand All @@ -429,12 +439,12 @@ class _InotifyFileSystemWatcher extends _FileSystemWatcher {

class _Win32FileSystemWatcher extends _FileSystemWatcher {
late StreamSubscription _subscription;
late StreamController _controller;
late StreamController<FileSystemEvent> _controller;

_Win32FileSystemWatcher(path, events, recursive)
: super._(path, events, recursive);

Stream _pathWatched() {
Stream<FileSystemEvent> _pathWatched() {
var pathId = _watcherPath!.pathId;
_controller = new StreamController<FileSystemEvent>();
_subscription =
Expand All @@ -457,12 +467,12 @@ class _Win32FileSystemWatcher extends _FileSystemWatcher {

class _FSEventStreamFileSystemWatcher extends _FileSystemWatcher {
late StreamSubscription _subscription;
late StreamController _controller;
late StreamController<FileSystemEvent> _controller;

_FSEventStreamFileSystemWatcher(path, events, recursive)
: super._(path, events, recursive);

Stream _pathWatched() {
Stream<FileSystemEvent> _pathWatched() {
var pathId = _watcherPath!.pathId;
var socketId = _FileSystemWatcher._getSocketId(0, pathId);
_controller = new StreamController<FileSystemEvent>();
Expand Down

0 comments on commit 9024e2b

Please sign in to comment.