[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: https://github.com/dart-lang/sdk/issues/60256 Change-Id: I0543ab26e5721a4048136f27e8f4429bef04920f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416300 Commit-Queue: Derek Xu <derekx@google.com> Reviewed-by: Ben Konyi <bkonyi@google.com>
2025-04-08 08:57:17 -07:00
parent ebe3c78630
commit 56ccf437e6
7 changed files with 285 additions and 52 deletions
@@ -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
@@ -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));
  }
@@ -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<void>();
+        process!.stdout.transform(utf8.decoder).listen((message) {
+          if (message.contains('Dart VM service no longer listening on ')) {
+            vmServiceShutDownCompleter.complete();
+          }
+        });
+        final vmServiceFailedToStartCompleter = Completer<void>();
+        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);
+}
@@ -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);
+}
@@ -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(() {
@@ -225,13 +225,35 @@ Future<List<Map<String, dynamic>>> listFilesCallback(Uri dirPath) async {

 Uri? serverInformationCallback() => server.serverAddress;

-Future<void> _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<bool> _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();
 }
@@ -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<bool> start(
+  /// Starts DDS.
+  ///
+  /// Throws a [_StartupException] if it fails to start.
+  Future<void> 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<String, dynamic>;
      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<void> startup() async {
    if (running) {
      // Already running.
@@ -303,7 +310,7 @@ class Server {
    final startingCompleter = Completer<bool>();
    _startingCompleter = startingCompleter;
    // Startup HTTP server.
-    Future<bool> startServer() async {
+    Future<void> 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());