Resolve process leak in test compiler (#2664)
Check for early closure after async operations and resolve a deadlock
during close where the frontend server is busy and is holding a pool
resource during shutdown.
Add a mechanism for injecting a fake `FrontendServerClient` to allow
controlling timing during a test without arbitrary delays trying line up
with the external process.
diff --git a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart
index b4eb512..618f473 100644
--- a/pkgs/test_core/lib/src/runner/vm/test_compiler.dart
+++ b/pkgs/test_core/lib/src/runner/vm/test_compiler.dart
@@ -46,9 +46,14 @@
/// A prefix used for the dill files for each compiler that is created.
final String _dillCachePrefix;
+ final FrontendClientFactory _clientFactory;
+
/// No work is done until the first call to [compile] is received, at which
/// point the compiler process is started.
- TestCompiler(this._dillCachePrefix);
+ TestCompiler(
+ this._dillCachePrefix, {
+ FrontendClientFactory clientFactory = FrontendServerClient.start,
+ }) : _clientFactory = clientFactory;
/// Compiles [mainDart], using a separate compiler per language version of
/// the tests.
@@ -62,6 +67,7 @@
() => _TestCompilerForLanguageVersion(
_dillCachePrefix,
languageVersionComment,
+ _clientFactory,
),
);
return compiler.compile(mainDart);
@@ -79,6 +85,7 @@
final _closeMemo = AsyncMemoizer<void>();
final _compilePool = Pool(1);
final String _dillCachePath;
+ final FrontendClientFactory _clientFactory;
FrontendServerClient? _frontendServerClient;
final String _languageVersionComment;
late final _outputDill = File(
@@ -96,6 +103,7 @@
_TestCompilerForLanguageVersion(
String dillCachePrefix,
this._languageVersionComment,
+ this._clientFactory,
) : _dillCachePath =
'$dillCachePrefix.'
'${_dillCacheSuffix(_languageVersionComment, enabledExperiments)}';
@@ -119,15 +127,16 @@
final testCache = File(_dillCachePath);
try {
- if (_frontendServerClient == null) {
+ if (_frontendServerClient case final frontendServerClient?) {
+ compilerOutput = await frontendServerClient.compile(<Uri>[
+ tempFile.uri,
+ ]);
+ } else {
if (await testCache.exists()) {
await testCache.copy(_outputDill.path);
}
compilerOutput = await _createCompiler(tempFile.uri);
- } else {
- compilerOutput = await _frontendServerClient!.compile(<Uri>[
- tempFile.uri,
- ]);
+ if (_closeMemo.hasRun) return CompilationResponse._wasShutdown;
}
} catch (e, s) {
if (_closeMemo.hasRun) return CompilationResponse._wasShutdown;
@@ -192,7 +201,7 @@
}
}
- var client = _frontendServerClient = await FrontendServerClient.start(
+ var client = await _clientFactory(
testUri.toString(),
_outputDill.path,
platformDill,
@@ -202,10 +211,19 @@
nativeAssets: nativeAssetsYaml?.toFilePath(),
printIncrementalDependencies: false,
);
+
+ if (_closeMemo.hasRun) {
+ client.kill();
+ return null;
+ }
+
+ _frontendServerClient = client;
return client.compile();
}
Future<void> dispose() => _closeMemo.runOnce(() async {
+ _frontendServerClient?.kill();
+ _frontendServerClient = null;
await _compilePool.close();
if (_dillToCache != null) {
var testCache = File(_dillCachePath);
@@ -214,14 +232,24 @@
}
_dillToCache!.copySync(_dillCachePath);
}
- _frontendServerClient?.kill();
- _frontendServerClient = null;
if (_outputDillDirectory.existsSync()) {
await _outputDillDirectory.deleteWithRetry();
}
});
}
+typedef FrontendClientFactory =
+ Future<FrontendServerClient> Function(
+ String entrypoint,
+ String outputDillPath,
+ String platformKernel, {
+ List<String>? enabledExperiments,
+ bool printIncrementalDependencies,
+ String sdkRoot,
+ String packagesJson,
+ String? nativeAssets,
+ });
+
/// Computes a unique dill cache suffix for each [languageVersionComment]
/// and [enabledExperiments] combination.
String _dillCacheSuffix(
diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml
index 37d3968..a69530c 100644
--- a/pkgs/test_core/pubspec.yaml
+++ b/pkgs/test_core/pubspec.yaml
@@ -34,3 +34,4 @@
dev_dependencies:
test: any
+ test_descriptor: ^2.0.0
diff --git a/pkgs/test_core/test/runner/vm/test_compiler_test.dart b/pkgs/test_core/test/runner/vm/test_compiler_test.dart
index b75a8b7..e220d3e 100644
--- a/pkgs/test_core/test/runner/vm/test_compiler_test.dart
+++ b/pkgs/test_core/test/runner/vm/test_compiler_test.dart
@@ -2,10 +2,17 @@
// 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:convert';
+import 'dart:io';
+import 'package:frontend_server_client/frontend_server_client.dart';
+import 'package:path/path.dart' as p;
+import 'package:test/fake.dart';
import 'package:test/test.dart';
+import 'package:test_api/backend.dart';
import 'package:test_core/src/runner/vm/test_compiler.dart';
+import 'package:test_descriptor/test_descriptor.dart' as d;
void main() {
group('VM test templates', () {
@@ -24,4 +31,170 @@
);
});
});
+
+ group('TestCompiler (integration)', () {
+ late String testPath;
+ late TestCompiler compiler;
+
+ setUp(() async {
+ await d.file('a_test.dart', 'void main() {}').create();
+ testPath = p.join(d.sandbox, 'a_test.dart');
+ compiler = TestCompiler(p.join(d.sandbox, 'dill_cache'));
+ });
+
+ tearDown(() async {
+ await compiler.dispose();
+ });
+
+ test('can compile a test', () async {
+ final response = await compiler.compile(Uri.file(testPath), Metadata());
+ expect(response.errorCount, 0);
+ expect(response.kernelOutputUri, isNotNull);
+ expect(File(response.kernelOutputUri!.toFilePath()).existsSync(), isTrue);
+ });
+ });
+
+ group('TestCompiler (with fakes)', () {
+ late String testPath;
+
+ setUp(() async {
+ await d.file('a_test.dart', 'void main() {}').create();
+ testPath = p.join(d.sandbox, 'a_test.dart');
+ });
+
+ test('can compile successfully with fake', () async {
+ final (fakeClient, clientStarter) = FakeFrontendServerClient.create;
+ final compiler = TestCompiler(
+ p.join(d.sandbox, 'dill_cache'),
+ clientFactory: clientStarter,
+ );
+
+ final compileFuture = compiler.compile(
+ Uri.file(testPath),
+ Metadata(languageVersionComment: '// @dart=3.0'),
+ );
+
+ await pumpEventQueue();
+
+ final outputDill = p.join(d.sandbox, 'output.dill');
+ File(outputDill).createSync();
+
+ fakeClient.completeCompile(
+ FakeCompileResult(
+ dillOutput: outputDill,
+ errorCount: 0,
+ compilerOutputLines: [],
+ ),
+ );
+
+ final response = await compileFuture;
+ expect(response.errorCount, 0);
+ expect(response.kernelOutputUri, isNotNull);
+
+ await compiler.dispose();
+ });
+
+ test('dispose kills active compiler and completes immediately', () async {
+ final (fakeClient, clientStarter) = FakeFrontendServerClient.create;
+ final compiler = TestCompiler(
+ p.join(d.sandbox, 'dill_cache'),
+ clientFactory: clientStarter,
+ );
+
+ final compileFuture = compiler.compile(
+ Uri.file(testPath),
+ Metadata(languageVersionComment: '// @dart=3.0'),
+ );
+
+ await pumpEventQueue();
+
+ expect(fakeClient.isCompileCalled, isTrue);
+ expect(fakeClient.isKilled, isFalse);
+
+ final disposeFuture = compiler.dispose();
+
+ await expectLater(disposeFuture, completes);
+ expect(fakeClient.isKilled, isTrue);
+
+ final response = await compileFuture;
+ expect(response.errorCount, 1);
+ expect(response.compilerOutput, contains('Compiler no longer active'));
+ });
+ });
+}
+
+class FakeCompileResult extends Fake implements CompileResult {
+ @override
+ final String? dillOutput;
+ @override
+ final int errorCount;
+ @override
+ final Iterable<String> compilerOutputLines;
+ @override
+ final Iterable<Uri> newSources;
+ @override
+ final Iterable<Uri> removedSources;
+
+ FakeCompileResult({
+ this.dillOutput,
+ this.errorCount = 0,
+ this.compilerOutputLines = const [],
+ this.newSources = const [],
+ this.removedSources = const [],
+ });
+}
+
+class FakeFrontendServerClient extends Fake implements FrontendServerClient {
+ var _compileCompleter = Completer<CompileResult>();
+ bool isKilled = false;
+ bool isCompileCalled = false;
+ int compileCallCount = 0;
+
+ static (FakeFrontendServerClient, FrontendClientFactory) get create {
+ final fakeClient = FakeFrontendServerClient();
+ return (
+ fakeClient,
+ (
+ _,
+ _,
+ _, {
+ List<String>? enabledExperiments,
+ bool printIncrementalDependencies = true,
+ String sdkRoot = '',
+ String packagesJson = '',
+ String? nativeAssets,
+ }) async => fakeClient,
+ );
+ }
+
+ @override
+ Future<CompileResult> compile([List<Uri>? sources]) {
+ isCompileCalled = true;
+ compileCallCount++;
+ if (_compileCompleter.isCompleted) {
+ _compileCompleter = Completer<CompileResult>();
+ }
+ return _compileCompleter.future;
+ }
+
+ void completeCompile(CompileResult result) {
+ if (!_compileCompleter.isCompleted) {
+ _compileCompleter.complete(result);
+ }
+ }
+
+ @override
+ bool kill({ProcessSignal processSignal = ProcessSignal.sigterm}) {
+ isKilled = true;
+ if (!_compileCompleter.isCompleted) {
+ _compileCompleter.completeError(StateError('Killed'));
+ }
+ return true;
+ }
+
+ @override
+ void accept() {}
+
+ @override
+ void reset() {}
}
