Skip to content

Commit 56ccf43

Browse files
derekxu16Commit Queue
authored andcommitted
[VM/Service] Shut down the VM immediately after the VM Service fails to start during VM initialization
TEST=pkg/vm_service/test/failure_to_start_vm_service_after_vm_is_initialized_test, pkg/vm_service/test/failure_to_start_vm_service_during_vm_initialization_test Fixes: #60256 Change-Id: I0543ab26e5721a4048136f27e8f4429bef04920f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416300 Commit-Queue: Derek Xu <[email protected]> Reviewed-by: Ben Konyi <[email protected]>
1 parent ebe3c78 commit 56ccf43

File tree

7 files changed

+285
-52
lines changed

7 files changed

+285
-52
lines changed

pkg/pkg.status

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ vm_service/test/enhanced_enum_test: SkipByDesign # Debugger is disabled in AOT m
138138
vm_service/test/eval_*test: SkipByDesign # Debugger is disabled in AOT mode.
139139
vm_service/test/evaluate_*test: SkipByDesign # Debugger is disabled in AOT mode.
140140
vm_service/test/external_compilation_service_test: SkipByDesign # Spawns a secondary process.
141+
vm_service/test/failure_to_start_vm_service_after_vm_is_initialized_test: SkipByDesign # Spawns a child process from source, and the codepath under test is the same in JIT and AOT.
142+
vm_service/test/failure_to_start_vm_service_during_vm_initialization_test: SkipByDesign # Spawns a child process from source, and the codepath under test is the same in JIT and AOT.
141143
vm_service/test/field_script_test: SkipByDesign # Debugger is disabled in AOT mode.
142144
vm_service/test/forward_compile_expression_error_from_external_client_with_dds_test: SkipByDesign # Debugger is disabled in AOT mode.
143145
vm_service/test/forward_compile_expression_error_from_external_client_without_dds_test: SkipByDesign # Debugger is disabled in AOT mode.
@@ -189,7 +191,7 @@ vm_service/test/regress_45684_test: SkipByDesign # Debugger is disabled in AOT m
189191
vm_service/test/regress_46419_test: SkipByDesign # Debugger is disabled in AOT mode.
190192
vm_service/test/regress_46559_test: SkipByDesign # Debugger is disabled in AOT mode.
191193
vm_service/test/regress_48279_test: SkipByDesign # Debugger is disabled in AOT mode.
192-
vm_service/test/regress_55559_test: SkipByDesign # Spawns a child process from source.
194+
vm_service/test/regress_55559_test: SkipByDesign # Spawns a child process from source, and the codepath under test is the same in JIT and AOT.
193195
vm_service/test/regress_60396_test: SkipByDesign # Debugger is disabled in AOT mode.
194196
vm_service/test/regress_88104_test: SkipByDesign # Debugger is disabled in AOT mode.
195197
vm_service/test/reload_sources_rpc_triggers_isolate_reload_event_test: SkipByDesign # Hot reload is disabled in AOT mode.
@@ -254,7 +256,6 @@ vm/test/*: SkipByDesign # Only meant to run on vm
254256
vm_snapshot_analysis/test/*: SkipByDesign # Only meant to run on vm
255257

256258
[ $system == windows ]
257-
_macros/test/executor/executor_test: Skip # dartbug.com/56002
258259
front_end/test/bootstrap_test: Skip # Issue 31902
259260
front_end/test/incremental_dart2js_load_from_dill_test: Pass, Slow
260261
vm_service/test/private_rpcs/dev_fs_http_put_test: Skip # Windows disallows "?" in paths

pkg/vm_service/test/common/utils.dart

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,18 @@ import 'dart:convert';
66
import 'dart:io';
77

88
// TODO(bkonyi): Share this logic with _ServiceTesteeRunner.launch.
9-
Future<(Process, Uri)> spawnDartProcess(
9+
Future<(Process, Uri?)> spawnDartProcess(
1010
String script, {
11+
bool enableDds = true,
12+
int vmServicePort = 0,
13+
14+
/// If true, the second element in the returned record will be a [Uri] that
15+
/// can be used to connect to the VM Service running on the testee. If false,
16+
/// the second element in the returned record will be null.
17+
bool returnServiceUri = true,
1118
bool serveObservatory = true,
12-
bool pauseOnStart = true,
19+
required bool pauseOnStart,
20+
required bool pauseOnExit,
1321
bool disableServiceAuthCodes = false,
1422
bool subscribeToStdio = true,
1523
}) async {
@@ -19,16 +27,18 @@ Future<(Process, Uri)> spawnDartProcess(
1927
final serviceInfoFile = await File.fromUri(serviceInfoUri).create();
2028

2129
final arguments = [
22-
'--no-dds',
23-
'--observe=0',
30+
if (!enableDds) '--no-dds',
31+
'--observe=$vmServicePort',
2432
if (!serveObservatory) '--no-serve-observatory',
2533
if (pauseOnStart) '--pause-isolates-on-start',
34+
if (pauseOnExit) '--pause-isolates-on-exit',
2635
if (disableServiceAuthCodes) '--disable-service-auth-codes',
2736
'--write-service-info=$serviceInfoUri',
2837
...Platform.executableArguments,
2938
Platform.script.resolve(script).toString(),
3039
];
3140
final process = await Process.start(executable, arguments);
41+
3242
if (subscribeToStdio) {
3343
process.stdout
3444
.transform(utf8.decoder)
@@ -37,6 +47,11 @@ Future<(Process, Uri)> spawnDartProcess(
3747
.transform(utf8.decoder)
3848
.listen((line) => print('TESTEE ERR: $line'));
3949
}
50+
51+
if (!returnServiceUri) {
52+
return (process, null);
53+
}
54+
4055
while ((await serviceInfoFile.length()) <= 5) {
4156
await Future.delayed(const Duration(milliseconds: 50));
4257
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Ensures that the VM does not shut down when an attempt to start the VM
6+
// Service via SIGQUIT fails. The VM Service SIGQUIT handler shares nearly all
7+
// of its code with dart:developer's controlWebServer, so this effecitvely tests
8+
// that function too.
9+
10+
import 'dart:async' show Completer;
11+
import 'dart:convert' show utf8;
12+
import 'dart:io'
13+
show HttpServer, InternetAddress, Platform, Process, ProcessSignal;
14+
15+
import 'package:test/test.dart';
16+
17+
import 'common/utils.dart';
18+
19+
void main() {
20+
HttpServer? server;
21+
Process? process;
22+
23+
tearDown(() async {
24+
await server?.close();
25+
server = null;
26+
process?.kill();
27+
process = null;
28+
});
29+
30+
void runTest({required final bool enableDds}) {
31+
test(
32+
'VM does not shut down when the VM Service fails to start after the VM '
33+
'is initialized${enableDds ? '' : ' with --disable-dds'}',
34+
() async {
35+
const vmServicePort = 8282;
36+
37+
final (spawnedProcess, _) = await spawnDartProcess(
38+
// We reuse 'sigquit_starts_service_script.dart' here because it just
39+
// waits in a loop.
40+
'sigquit_starts_service_script.dart',
41+
enableDds: enableDds,
42+
vmServicePort: vmServicePort,
43+
pauseOnStart: false,
44+
pauseOnExit: true,
45+
subscribeToStdio: false,
46+
);
47+
process = spawnedProcess;
48+
49+
// Listen for the messages that should be printed when we toggle the VM
50+
// Service.
51+
final vmServiceShutDownCompleter = Completer<void>();
52+
process!.stdout.transform(utf8.decoder).listen((message) {
53+
if (message.contains('Dart VM service no longer listening on ')) {
54+
vmServiceShutDownCompleter.complete();
55+
}
56+
});
57+
final vmServiceFailedToStartCompleter = Completer<void>();
58+
process!.stderr.transform(utf8.decoder).listen((message) {
59+
if (message.contains('Could not start the VM service')) {
60+
vmServiceFailedToStartCompleter.complete();
61+
}
62+
});
63+
64+
// Shut down the VM Service running in the testee.
65+
process!.kill(ProcessSignal.sigquit);
66+
await vmServiceShutDownCompleter.future;
67+
// Wait a bit more to make sure that [vmServicePort] is free.
68+
await Future.delayed(const Duration(seconds: 3));
69+
70+
// Bind an HTTP server to [vmServicePort].
71+
server = await HttpServer.bind(
72+
InternetAddress.loopbackIPv4,
73+
vmServicePort,
74+
);
75+
76+
// Try restarting the VM Service running in the testee. This should fail
77+
// because [server] is bound to [vmServicePort].
78+
process!.kill(ProcessSignal.sigquit);
79+
await vmServiceFailedToStartCompleter.future;
80+
81+
process!.kill();
82+
// Check that the process only exited after receiving SIGTERM, and not
83+
// when the VM Service failed to start.
84+
expect(await process!.exitCode, -ProcessSignal.sigterm.signalNumber);
85+
},
86+
skip: Platform.isWindows,
87+
);
88+
}
89+
90+
runTest(enableDds: true);
91+
runTest(enableDds: false);
92+
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Regression test for https://github.com/dart-lang/sdk/issues/60256.
6+
//
7+
// Ensures that the VM shuts down immediately after the VM Service fails to
8+
// start during VM initialization. The specific problem that motivated this test
9+
// was that the VM used to get stuck paused at exit when the VM Service failed
10+
// to start during initialization and `--pause-isolates-on-exit` was supplied.
11+
12+
import 'dart:convert' show utf8;
13+
import 'dart:io' show HttpServer, InternetAddress, Process;
14+
15+
import 'package:test/test.dart';
16+
17+
import 'common/utils.dart';
18+
19+
void main() {
20+
HttpServer? server;
21+
Process? process;
22+
23+
tearDown(() async {
24+
await server?.close();
25+
server = null;
26+
process?.kill();
27+
process = null;
28+
});
29+
30+
void runTest({required final bool enableDds}) {
31+
test(
32+
'Regress 60256: VM shuts down immediately after the VM Service fails to '
33+
'start during VM initialization${enableDds ? '' : ' with --disable-dds'}',
34+
() async {
35+
server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0);
36+
37+
// Force the testee VM Service to fail to start by making it try to bind
38+
// to the same address [server] is already running on.
39+
final (spawnedProcess, _) = await spawnDartProcess(
40+
// We expect the VM to shut down before running the script, so we just
41+
// pass an arbitrary script here.
42+
'regress_55559_script.dart',
43+
enableDds: enableDds,
44+
vmServicePort: server!.port,
45+
returnServiceUri: false,
46+
pauseOnStart: false,
47+
pauseOnExit: true,
48+
subscribeToStdio: false,
49+
);
50+
process = spawnedProcess;
51+
52+
final first = utf8.decode(await process!.stderr.first);
53+
expect(
54+
first,
55+
allOf(
56+
contains('Could not start the VM service'),
57+
contains(
58+
'Failed to create server socket',
59+
),
60+
),
61+
);
62+
// Ensure that the VM terminates instead of hanging.
63+
final exitCode = await process!.exitCode;
64+
// 255 is the value of kErrorExitCode in runtime/bin/error_exit.h.
65+
expect(exitCode, 255);
66+
},
67+
);
68+
}
69+
70+
runTest(enableDds: true);
71+
runTest(enableDds: false);
72+
}

pkg/vm_service/test/regress_55559_test.dart

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,20 @@ void main() {
3030
}
3131

3232
setUp(() async {
33-
state = await spawnDartProcess(
33+
switch (await spawnDartProcess(
3434
'regress_55559_script.dart',
35+
enableDds: false,
3536
pauseOnStart: false,
36-
);
37+
pauseOnExit: false,
38+
)) {
39+
case (final Process process, final Uri uri):
40+
state = (process, uri);
41+
default:
42+
fail(
43+
"The implementation of spawnDartProcess's returnServiceUri parameter"
44+
'is incorrect',
45+
);
46+
}
3747
});
3848

3949
tearDown(() {

sdk/lib/_internal/vm/bin/vmservice_io.dart

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,35 @@ Future<List<Map<String, dynamic>>> listFilesCallback(Uri dirPath) async {
225225

226226
Uri? serverInformationCallback() => server.serverAddress;
227227

228-
Future<void> _toggleWebServer() async {
228+
/// Thrown when either the VM Service HTTP server or DDS fails to start.
229+
class _StartupException implements Exception {
230+
final String message;
231+
232+
_StartupException(this.message);
233+
}
234+
235+
/// Toggles the running state of the VM Service HTTP server. If
236+
/// [server._waitForDdsToAdvertiseService] is true, toggles DDS alongside the VM
237+
/// Service HTTP server.
238+
///
239+
/// Logs error messages to stderr and completes the returned [Future] with
240+
/// [false] if an attempt is made to enable the VM Service HTTP server or DDS
241+
/// and the attempt fails. Completes the returned [Future] with [true]
242+
/// otherwise.
243+
Future<bool> _toggleWebServer() async {
229244
// Toggle HTTP server.
230245
if (server.running) {
231246
await server.shutdown(true);
232247
await VMService().clearState();
248+
return true;
233249
} else {
234-
await server.startup();
250+
try {
251+
await server.startup();
252+
return true;
253+
} on _StartupException catch (e) {
254+
stderr.writeln(e.message);
255+
return false;
256+
}
235257
}
236258
}
237259

@@ -267,7 +289,7 @@ void _registerSignalHandler() {
267289
}
268290
_signalSubscription = signalWatch(
269291
ProcessSignal.sigquit,
270-
).listen((_) => _toggleWebServer());
292+
).listen((_) => unawaited(_toggleWebServer()));
271293
}
272294

273295
@pragma('vm:entry-point', !bool.fromEnvironment('dart.vm.product'))
@@ -303,7 +325,13 @@ void main() {
303325
);
304326

305327
if (_autoStart) {
306-
_toggleWebServer();
328+
unawaited(
329+
_toggleWebServer().then((wasSuccessful) {
330+
if (!wasSuccessful) {
331+
exit(vmErrorExitCode);
332+
}
333+
}),
334+
);
307335
}
308336
_registerSignalHandler();
309337
}

0 commit comments

Comments
 (0)