[ Service ] Fix race in startup / shutdown code

Fixes https://github.com/dart-lang/sdk/issues/56606

TEST=regress_56606_test.dart

CoreLibraryReviewExempt: VM service only change
Change-Id: Ie3a96d277547ac8ef4bf45ba700ad311d5997817
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/383060
Reviewed-by: Derek Xu <derekx@google.com>
Auto-Submit: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/dartdev/test/regress_56606_test.dart

@@ -0,0 +1,53 @@
+// Copyright (c) 2024, 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.
+
+import 'dart:io';
+
+import 'package:path/path.dart' as path;
+import 'package:test/test.dart';
+import 'utils.dart';
+
+const numRuns = 10;
+
+final smokeTestScript = r'''
+void main() {
+  print('Smoke test!');
+}
+''';
+
+void main() {
+  late final String script;
+  late final TestProject p;
+
+  setUp(() {
+    p = project(mainSrc: smokeTestScript);
+    script = path.join(p.dirPath, p.relativeFilePath);
+  });
+
+  tearDown(() async {
+    await p.dispose();
+  });
+
+  test(
+    'Regression test for https://github.com/dart-lang/sdk/issues/56606',
+    () async {
+      // Tests for a race condition in the VM service startup / shutdown logic
+      // that previously caused crashes when the VM service was enabled for
+      // programs with short lifespans.
+      for (int i = 1; i <= numRuns; ++i) {
+        if (i % 5 == 0) {
+          print('Done [$i/$numRuns]');
+        }
+        final result = await Process.run(
+          Platform.executable,
+          ['--enable-vm-service', script],
+          environment: {'BOT': '1'},
+        );
+        expect(result.stderr, isEmpty);
+        expect(result.stdout, contains('Smoke test!'));
+        expect(result.exitCode, 0);
+      }
+    },
+  );
+}

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

@@ -96,7 +96,7 @@ Future<void> cleanupCallback() async {
   final localServer = server;
   if (localServer != null) {
     try {
-      await localServer.cleanup(true);
+      await localServer.shutdown(true);
     } catch (e, st) {
       print('Error in vm-service shutdown: $e\n$st\n');
     }

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

@@ -216,10 +216,7 @@ class _DebuggingSession {
     return true;
   }
 
-  void shutdown() {
-    print('Shutting down DDS!\n${StackTrace.current}');
-    _process!.kill();
-  }
+  void shutdown() => _process!.kill();
 
   Process? _process;
 }
@@ -635,22 +632,15 @@ class Server {
     }
   }
 
-  Future<void> cleanup(bool force) {
-    final serverLocal = _server;
-    if (serverLocal == null) {
-      return Future.value();
-    }
-    if (Platform.isFuchsia) {
-      // Remove the file with the port number.
-      final tmp = Directory.systemTemp.path;
-      final path = '$tmp/dart.services/${serverLocal.port}';
-      serverPrint('Deleting $path');
-      File(path)..deleteSync();
-    }
-    return serverLocal.close(force: force);
+  void _cleanupFuchsiaState(int port) {
+    // Remove the file with the port number.
+    final tmp = Directory.systemTemp.path;
+    final path = '$tmp/dart.services/$port';
+    serverPrint('Deleting $path');
+    File(path).deleteSync();
   }
 
-  Future<Server> shutdown(bool forced) async {
+  Future<void> shutdown(bool forced) async {
     // If start is pending, wait for it to complete.
     if (_startingCompleter != null) {
       if (!_startingCompleter!.isCompleted) {
@@ -658,32 +648,37 @@ class Server {
       }
     }
 
-    if (_server == null) {
+    final server = _server;
+    if (server == null) {
       // Not started.
-      return Future.value(this);
+      return;
     }
 
-    // Shutdown HTTP server and subscription.
-    Uri oldServerAddress = serverAddress!;
-    return cleanup(forced).then((_) {
-      serverPrint('Dart VM service no longer listening on $oldServerAddress');
-      _server = null;
-      _startingCompleter = null;
+    if (Platform.isFuchsia) {
+      _cleanupFuchsiaState(server.port);
+    }
+
+    final address = serverAddress!;
+
+    try {
+      // Shutdown HTTP server and subscription.
+      await server.close(force: forced);
+      if (!_service.isExiting) {
+        // Only print this message if the service has been toggled off, not
+        // when the VM is exiting.
+        serverPrint('Dart VM service no longer listening on $address');
+      }
+    } catch (e, st) {
+      serverPrint('Could not shutdown Dart VM service HTTP server:\n$e\n$st\n');
+    } finally {
       _ddsInstance?.shutdown();
       _ddsInstance = null;
-      _running = false;
-      _notifyServerState('');
-      onServerAddressChange(null);
-      return this;
-    }).catchError((e, st) {
       _server = null;
       _startingCompleter = null;
-      serverPrint('Could not shutdown Dart VM service HTTP server:\n$e\n$st\n');
       _running = false;
       _notifyServerState('');
       onServerAddressChange(null);
-      return this;
-    });
+    }
   }
 }
 

sdk/lib/vmservice/vmservice.dart

@@ -476,12 +476,11 @@ class VMService extends MessageRouter {
     // Close receive ports.
     isolateControlPort.close();
     scriptLoadPort.close();
-    await clearState();
     final cleanup = VMServiceEmbedderHooks.cleanup;
     if (cleanup != null) {
       await cleanup();
     }
-
+    await clearState();
     // Notify the VM that we have exited.
     _onExit();
   }