Skip to content

Commit c2095cb

Browse files
derekxu16Commit Queue
authored andcommitted
Reland "[VM/Service] Shut down the VM immediately after the VM Service fails to start during VM initialization"
This reverts commit d510876. Reason for revert: g3 and Golem have been made compatible with this CL. See b/409535026 and https://chrome-internal-review.googlesource.com/c/golem/+/8345341. 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 Original change's description: > Revert "[VM/Service] Shut down the VM immediately after the VM Service fails to start during VM initialization" > > This reverts commit 56ccf43. > > Reason for revert: b/409535026 > > TEST=ci > > Original change's description: > > [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]> > > Change-Id: I61eb42f0f00ad97e95e3ebf19990fe75d2d416aa > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421622 > Reviewed-by: Derek Xu <[email protected]> > Commit-Queue: Ivan Inozemtsev <[email protected]> > Reviewed-by: Ben Konyi <[email protected]> Change-Id: Ieba880b7b298e491055c3f6049bab6772b1f8aa3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/434960 Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Derek Xu <[email protected]>
1 parent 76997e6 commit c2095cb

File tree

7 files changed

+285
-50
lines changed

7 files changed

+285
-50
lines changed

pkg/pkg.status

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ vm_service/test/enhanced_enum_test: SkipByDesign # Debugger is disabled in AOT m
139139
vm_service/test/eval_*test: SkipByDesign # Debugger is disabled in AOT mode.
140140
vm_service/test/evaluate_*test: SkipByDesign # Debugger is disabled in AOT mode.
141141
vm_service/test/external_compilation_service_test: SkipByDesign # Spawns a secondary process.
142+
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.
143+
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.
142144
vm_service/test/field_script_test: SkipByDesign # Debugger is disabled in AOT mode.
143145
vm_service/test/forward_compile_expression_error_from_external_client_with_dds_test: SkipByDesign # Debugger is disabled in AOT mode.
144146
vm_service/test/forward_compile_expression_error_from_external_client_without_dds_test: SkipByDesign # Debugger is disabled in AOT mode.
@@ -191,7 +193,7 @@ vm_service/test/regress_45684_test: SkipByDesign # Debugger is disabled in AOT m
191193
vm_service/test/regress_46419_test: SkipByDesign # Debugger is disabled in AOT mode.
192194
vm_service/test/regress_46559_test: SkipByDesign # Debugger is disabled in AOT mode.
193195
vm_service/test/regress_48279_test: SkipByDesign # Debugger is disabled in AOT mode.
194-
vm_service/test/regress_55559_test: SkipByDesign # Spawns a child process from source.
196+
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.
195197
vm_service/test/regress_60396_test: SkipByDesign # Debugger is disabled in AOT mode.
196198
vm_service/test/regress_88104_test: SkipByDesign # Debugger is disabled in AOT mode.
197199
vm_service/test/reload_sources_rpc_triggers_isolate_reload_event_test: SkipByDesign # Hot reload is disabled in AOT mode.

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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,21 @@ void main() {
3030
}
3131

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

3950
tearDown(() {

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

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

221221
Uri? serverInformationCallback() => server.serverAddress;
222222

223-
Future<void> _toggleWebServer() async {
223+
/// Thrown when either the VM Service HTTP server or DDS fails to start.
224+
class _StartupException implements Exception {
225+
final String message;
226+
227+
_StartupException(this.message);
228+
}
229+
230+
/// Toggles the running state of the VM Service HTTP server. If
231+
/// [server._waitForDdsToAdvertiseService] is true, toggles DDS alongside the VM
232+
/// Service HTTP server.
233+
///
234+
/// Logs error messages to stderr and completes the returned [Future] with
235+
/// [false] if an attempt is made to enable the VM Service HTTP server or DDS
236+
/// and the attempt fails. Completes the returned [Future] with [true]
237+
/// otherwise.
238+
Future<bool> _toggleWebServer() async {
224239
// Toggle HTTP server.
225240
if (server.running) {
226241
await server.shutdown(true);
227242
await VMService().clearState();
243+
return true;
228244
} else {
229-
await server.startup();
245+
try {
246+
await server.startup();
247+
return true;
248+
} on _StartupException catch (e) {
249+
stderr.writeln(e.message);
250+
return false;
251+
}
230252
}
231253
}
232254

@@ -262,7 +284,7 @@ void _registerSignalHandler() {
262284
}
263285
_signalSubscription = signalWatch(
264286
ProcessSignal.sigquit,
265-
).listen((_) => _toggleWebServer());
287+
).listen((_) => unawaited(_toggleWebServer()));
266288
}
267289

268290
@pragma('vm:entry-point', !bool.fromEnvironment('dart.vm.product'))
@@ -297,7 +319,13 @@ void main() {
297319
);
298320

299321
if (_autoStart) {
300-
_toggleWebServer();
322+
unawaited(
323+
_toggleWebServer().then((wasSuccessful) {
324+
if (!wasSuccessful) {
325+
exit(vmErrorExitCode);
326+
}
327+
}),
328+
);
301329
}
302330
_registerSignalHandler();
303331
}

0 commit comments

Comments
 (0)