diff --git a/pkg/pkg.status b/pkg/pkg.status index c82e56b2db9..6024ab302bd 100644 --- a/pkg/pkg.status +++ b/pkg/pkg.status @@ -138,6 +138,8 @@ vm_service/test/enhanced_enum_test: SkipByDesign # Debugger is disabled in AOT m vm_service/test/eval_*test: SkipByDesign # Debugger is disabled in AOT mode. vm_service/test/evaluate_*test: SkipByDesign # Debugger is disabled in AOT mode. vm_service/test/external_compilation_service_test: SkipByDesign # Spawns a secondary process. +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. +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. vm_service/test/field_script_test: SkipByDesign # Debugger is disabled in AOT mode. vm_service/test/forward_compile_expression_error_from_external_client_with_dds_test: SkipByDesign # Debugger is disabled in AOT mode. 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 vm_service/test/regress_46419_test: SkipByDesign # Debugger is disabled in AOT mode. vm_service/test/regress_46559_test: SkipByDesign # Debugger is disabled in AOT mode. vm_service/test/regress_48279_test: SkipByDesign # Debugger is disabled in AOT mode. -vm_service/test/regress_55559_test: SkipByDesign # Spawns a child process from source. +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. vm_service/test/regress_60396_test: SkipByDesign # Debugger is disabled in AOT mode. vm_service/test/regress_88104_test: SkipByDesign # Debugger is disabled in AOT mode. 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 vm_snapshot_analysis/test/*: SkipByDesign # Only meant to run on vm [ $system == windows ] -_macros/test/executor/executor_test: Skip # dartbug.com/56002 front_end/test/bootstrap_test: Skip # Issue 31902 front_end/test/incremental_dart2js_load_from_dill_test: Pass, Slow vm_service/test/private_rpcs/dev_fs_http_put_test: Skip # Windows disallows "?" in paths diff --git a/pkg/vm_service/test/common/utils.dart b/pkg/vm_service/test/common/utils.dart index 0d3b2c6468a..4853445607a 100644 --- a/pkg/vm_service/test/common/utils.dart +++ b/pkg/vm_service/test/common/utils.dart @@ -6,10 +6,18 @@ import 'dart:convert'; import 'dart:io'; // TODO(bkonyi): Share this logic with _ServiceTesteeRunner.launch. -Future<(Process, Uri)> spawnDartProcess( +Future<(Process, Uri?)> spawnDartProcess( String script, { + bool enableDds = true, + int vmServicePort = 0, + + /// If true, the second element in the returned record will be a [Uri] that + /// can be used to connect to the VM Service running on the testee. If false, + /// the second element in the returned record will be null. + bool returnServiceUri = true, bool serveObservatory = true, - bool pauseOnStart = true, + required bool pauseOnStart, + required bool pauseOnExit, bool disableServiceAuthCodes = false, bool subscribeToStdio = true, }) async { @@ -19,16 +27,18 @@ Future<(Process, Uri)> spawnDartProcess( final serviceInfoFile = await File.fromUri(serviceInfoUri).create(); final arguments = [ - '--no-dds', - '--observe=0', + if (!enableDds) '--no-dds', + '--observe=$vmServicePort', if (!serveObservatory) '--no-serve-observatory', if (pauseOnStart) '--pause-isolates-on-start', + if (pauseOnExit) '--pause-isolates-on-exit', if (disableServiceAuthCodes) '--disable-service-auth-codes', '--write-service-info=$serviceInfoUri', ...Platform.executableArguments, Platform.script.resolve(script).toString(), ]; final process = await Process.start(executable, arguments); + if (subscribeToStdio) { process.stdout .transform(utf8.decoder) @@ -37,6 +47,11 @@ Future<(Process, Uri)> spawnDartProcess( .transform(utf8.decoder) .listen((line) => print('TESTEE ERR: $line')); } + + if (!returnServiceUri) { + return (process, null); + } + while ((await serviceInfoFile.length()) <= 5) { await Future.delayed(const Duration(milliseconds: 50)); } diff --git a/pkg/vm_service/test/failure_to_start_vm_service_after_vm_is_initialized_test.dart b/pkg/vm_service/test/failure_to_start_vm_service_after_vm_is_initialized_test.dart new file mode 100644 index 00000000000..fd7c46d6489 --- /dev/null +++ b/pkg/vm_service/test/failure_to_start_vm_service_after_vm_is_initialized_test.dart @@ -0,0 +1,92 @@ +// Copyright (c) 2025, 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. + +// Ensures that the VM does not shut down when an attempt to start the VM +// Service via SIGQUIT fails. The VM Service SIGQUIT handler shares nearly all +// of its code with dart:developer's controlWebServer, so this effecitvely tests +// that function too. + +import 'dart:async' show Completer; +import 'dart:convert' show utf8; +import 'dart:io' + show HttpServer, InternetAddress, Platform, Process, ProcessSignal; + +import 'package:test/test.dart'; + +import 'common/utils.dart'; + +void main() { + HttpServer? server; + Process? process; + + tearDown(() async { + await server?.close(); + server = null; + process?.kill(); + process = null; + }); + + void runTest({required final bool enableDds}) { + test( + 'VM does not shut down when the VM Service fails to start after the VM ' + 'is initialized${enableDds ? '' : ' with --disable-dds'}', + () async { + const vmServicePort = 8282; + + final (spawnedProcess, _) = await spawnDartProcess( + // We reuse 'sigquit_starts_service_script.dart' here because it just + // waits in a loop. + 'sigquit_starts_service_script.dart', + enableDds: enableDds, + vmServicePort: vmServicePort, + pauseOnStart: false, + pauseOnExit: true, + subscribeToStdio: false, + ); + process = spawnedProcess; + + // Listen for the messages that should be printed when we toggle the VM + // Service. + final vmServiceShutDownCompleter = Completer(); + process!.stdout.transform(utf8.decoder).listen((message) { + if (message.contains('Dart VM service no longer listening on ')) { + vmServiceShutDownCompleter.complete(); + } + }); + final vmServiceFailedToStartCompleter = Completer(); + process!.stderr.transform(utf8.decoder).listen((message) { + if (message.contains('Could not start the VM service')) { + vmServiceFailedToStartCompleter.complete(); + } + }); + + // Shut down the VM Service running in the testee. + process!.kill(ProcessSignal.sigquit); + await vmServiceShutDownCompleter.future; + // Wait a bit more to make sure that [vmServicePort] is free. + await Future.delayed(const Duration(seconds: 3)); + + // Bind an HTTP server to [vmServicePort]. + server = await HttpServer.bind( + InternetAddress.loopbackIPv4, + vmServicePort, + ); + + // Try restarting the VM Service running in the testee. This should fail + // because [server] is bound to [vmServicePort]. + process!.kill(ProcessSignal.sigquit); + await vmServiceFailedToStartCompleter.future; + + process!.kill(); + // Check that the process only exited after receiving SIGTERM, and not + // when the VM Service failed to start. + expect(await process!.exitCode, -ProcessSignal.sigterm.signalNumber); + }, + skip: Platform.isWindows, + ); + } + + runTest(enableDds: true); + runTest(enableDds: false); +} diff --git a/pkg/vm_service/test/failure_to_start_vm_service_during_vm_initialization_test.dart b/pkg/vm_service/test/failure_to_start_vm_service_during_vm_initialization_test.dart new file mode 100644 index 00000000000..25f96368354 --- /dev/null +++ b/pkg/vm_service/test/failure_to_start_vm_service_during_vm_initialization_test.dart @@ -0,0 +1,72 @@ +// Copyright (c) 2025, 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. + +// Regression test for https://github.com/dart-lang/sdk/issues/60256. +// +// Ensures that the VM shuts down immediately after the VM Service fails to +// start during VM initialization. The specific problem that motivated this test +// was that the VM used to get stuck paused at exit when the VM Service failed +// to start during initialization and `--pause-isolates-on-exit` was supplied. + +import 'dart:convert' show utf8; +import 'dart:io' show HttpServer, InternetAddress, Process; + +import 'package:test/test.dart'; + +import 'common/utils.dart'; + +void main() { + HttpServer? server; + Process? process; + + tearDown(() async { + await server?.close(); + server = null; + process?.kill(); + process = null; + }); + + void runTest({required final bool enableDds}) { + test( + 'Regress 60256: VM shuts down immediately after the VM Service fails to ' + 'start during VM initialization${enableDds ? '' : ' with --disable-dds'}', + () async { + server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0); + + // Force the testee VM Service to fail to start by making it try to bind + // to the same address [server] is already running on. + final (spawnedProcess, _) = await spawnDartProcess( + // We expect the VM to shut down before running the script, so we just + // pass an arbitrary script here. + 'regress_55559_script.dart', + enableDds: enableDds, + vmServicePort: server!.port, + returnServiceUri: false, + pauseOnStart: false, + pauseOnExit: true, + subscribeToStdio: false, + ); + process = spawnedProcess; + + final first = utf8.decode(await process!.stderr.first); + expect( + first, + allOf( + contains('Could not start the VM service'), + contains( + 'Failed to create server socket', + ), + ), + ); + // Ensure that the VM terminates instead of hanging. + final exitCode = await process!.exitCode; + // 255 is the value of kErrorExitCode in runtime/bin/error_exit.h. + expect(exitCode, 255); + }, + ); + } + + runTest(enableDds: true); + runTest(enableDds: false); +} diff --git a/pkg/vm_service/test/regress_55559_test.dart b/pkg/vm_service/test/regress_55559_test.dart index 57c4f745192..008f8936020 100644 --- a/pkg/vm_service/test/regress_55559_test.dart +++ b/pkg/vm_service/test/regress_55559_test.dart @@ -30,10 +30,20 @@ void main() { } setUp(() async { - state = await spawnDartProcess( + switch (await spawnDartProcess( 'regress_55559_script.dart', + enableDds: false, pauseOnStart: false, - ); + pauseOnExit: false, + )) { + case (final Process process, final Uri uri): + state = (process, uri); + default: + fail( + "The implementation of spawnDartProcess's returnServiceUri parameter" + 'is incorrect', + ); + } }); tearDown(() { diff --git a/sdk/lib/_internal/vm/bin/vmservice_io.dart b/sdk/lib/_internal/vm/bin/vmservice_io.dart index 32a38b54fd6..dc61b3b9e8b 100644 --- a/sdk/lib/_internal/vm/bin/vmservice_io.dart +++ b/sdk/lib/_internal/vm/bin/vmservice_io.dart @@ -225,13 +225,35 @@ Future>> listFilesCallback(Uri dirPath) async { Uri? serverInformationCallback() => server.serverAddress; -Future _toggleWebServer() async { +/// Thrown when either the VM Service HTTP server or DDS fails to start. +class _StartupException implements Exception { + final String message; + + _StartupException(this.message); +} + +/// Toggles the running state of the VM Service HTTP server. If +/// [server._waitForDdsToAdvertiseService] is true, toggles DDS alongside the VM +/// Service HTTP server. +/// +/// Logs error messages to stderr and completes the returned [Future] with +/// [false] if an attempt is made to enable the VM Service HTTP server or DDS +/// and the attempt fails. Completes the returned [Future] with [true] +/// otherwise. +Future _toggleWebServer() async { // Toggle HTTP server. if (server.running) { await server.shutdown(true); await VMService().clearState(); + return true; } else { - await server.startup(); + try { + await server.startup(); + return true; + } on _StartupException catch (e) { + stderr.writeln(e.message); + return false; + } } } @@ -267,7 +289,7 @@ void _registerSignalHandler() { } _signalSubscription = signalWatch( ProcessSignal.sigquit, - ).listen((_) => _toggleWebServer()); + ).listen((_) => unawaited(_toggleWebServer())); } @pragma('vm:entry-point', !bool.fromEnvironment('dart.vm.product')) @@ -303,7 +325,13 @@ void main() { ); if (_autoStart) { - _toggleWebServer(); + unawaited( + _toggleWebServer().then((wasSuccessful) { + if (!wasSuccessful) { + exit(vmErrorExitCode); + } + }), + ); } _registerSignalHandler(); } diff --git a/sdk/lib/_internal/vm/bin/vmservice_server.dart b/sdk/lib/_internal/vm/bin/vmservice_server.dart index 3774d0f773f..49d5c0f1b09 100644 --- a/sdk/lib/_internal/vm/bin/vmservice_server.dart +++ b/sdk/lib/_internal/vm/bin/vmservice_server.dart @@ -4,6 +4,9 @@ part of vmservice_io; +// This must be kept in sync with kErrorExitCode in runtime/bin/error_exit.h. +const vmErrorExitCode = 255; + // TODO(48602): deprecate SILENT_OBSERVATORY in favor of SILENT_VM_SERVICE bool silentObservatory = bool.fromEnvironment('SILENT_OBSERVATORY'); bool silentVMService = bool.fromEnvironment('SILENT_VM_SERVICE'); @@ -142,13 +145,20 @@ class HttpRequestClient extends Client { /// Responsible for launching a DevTools instance when the service is started /// via SIGQUIT. class _DebuggingSession { - Future start( + /// Starts DDS. + /// + /// Throws a [_StartupException] if it fails to start. + Future start( Uri serverAddress, String host, String port, bool disableServiceAuthCodes, bool enableDevTools, ) async { + void _throwStartupException(String details) { + throw _StartupException('Could not start the VM service:\n$details'); + } + // This code is part of the SDK and it is ok to have a reference to the // internals of the Dart SDK in terms of location of the snapshot etc. // It is more efficient doing it this way instead of invoking the Dart CLI @@ -180,27 +190,23 @@ class _DebuggingSession { FileSystemEntityType.notFound) { executable = dart; } - var process = await Process.start(executable, [ - script, - '--vm-service-uri=$serverAddress', - '--bind-address=$host', - '--bind-port=$port', - if (disableServiceAuthCodes) '--disable-service-auth-codes', - if (enableDevTools) '--serve-devtools', - if (_enableServicePortFallback) '--enable-service-port-fallback', - ], mode: ProcessStartMode.detachedWithStdio); - if (process == null) { - stderr.writeln('Could not start the VM service: Process.start failed\n'); - return false; + try { + _process = await Process.start(executable, [ + script, + '--vm-service-uri=$serverAddress', + '--bind-address=$host', + '--bind-port=$port', + if (disableServiceAuthCodes) '--disable-service-auth-codes', + if (enableDevTools) '--serve-devtools', + if (_enableServicePortFallback) '--enable-service-port-fallback', + ], mode: ProcessStartMode.detachedWithStdio); + } on ProcessException catch (e) { + _throwStartupException('Process.start failed: ${e.message}'); } - _process = process; // DDS will close stderr once it's finished launching. final launchResult = await _process.stderr.transform(utf8.decoder).join(); - void printError(String details) => - stderr.writeln('Could not start the VM service:\n$details'); - try { final result = json.decode(launchResult) as Map; if (result case {'state': 'started'}) { @@ -215,16 +221,13 @@ class _DebuggingSession { serverPrint('The Dart Tooling Daemon (DTD) is available at: $dtdUri'); } } else { - printError(result['error'] ?? result); - return false; + _throwStartupException(result['error'] ?? result); } } catch (_) { // Malformed JSON was likely encountered, so output the entirety of // stderr in the error message. - printError(launchResult); - return false; + _throwStartupException(launchResult); } - return true; } void shutdown() => _process.kill(); @@ -284,6 +287,10 @@ class Server { this._enableServicePortFallback, ) : _authCodesDisabled = (authCodesDisabled || Platform.isFuchsia); + /// Starts the VM Service HTTP server. If [_waitForDdsToAdvertiseService] is + /// true, starts DDS as well. + /// + /// Throws a [_StartupException] if either of them fails to start. Future startup() async { if (running) { // Already running. @@ -303,7 +310,7 @@ class Server { final startingCompleter = Completer(); _startingCompleter = startingCompleter; // Startup HTTP server. - Future startServer() async { + Future startServer() async { try { var address; var addresses = await InternetAddress.lookup(_ip); @@ -322,22 +329,17 @@ class Server { _port = 0; return await startServer(); } else { - serverPrint( - 'Could not start Dart VM service HTTP server:\n' + startingCompleter.complete(true); + _startingCompleter = null; + throw _StartupException( + 'Could not start the VM service HTTP server:\n' '$e\n$st', ); - _notifyServerState(''); - onServerAddressChange(null); - return false; } } - return true; } - if (!(await startServer())) { - startingCompleter.complete(true); - return; - } + await startServer(); if (_service.isExiting) { serverPrint( 'Dart VM service HTTP server exiting before listening as ' @@ -352,16 +354,29 @@ class Server { if (_waitForDdsToAdvertiseService) { _ddsInstance = _DebuggingSession(); - await _ddsInstance!.start( - serverAddress!, - _ddsIP, - _ddsPort.toString(), - _authCodesDisabled, - _serveDevtools, - ); + try { + await _ddsInstance!.start( + serverAddress!, + _ddsIP, + _ddsPort.toString(), + _authCodesDisabled, + _serveDevtools, + ); + } on _StartupException { + // If DDS fails to start, shut down the HTTP server as well. + try { + await server.close(force: true); + } catch (_) {} + _ddsInstance = null; + _httpServer = null; + startingCompleter.complete(true); + _startingCompleter = null; + rethrow; + } } else { await outputConnectionInformation(); } + // Server is up and running. _running = true; _notifyServerState(serverAddress.toString());