[vm/vmservice] Ensure only one vmservice http server is launched.

When many concurrent requests come in, it was possible to leak http servers, leading to hanging dart vms.

Fixes dart-lang/sdk#50389
TEST=developer_server_launch_test

Change-Id: Icc59987a1a60af5ec72e0cb1ca7b43dea7f0c5e3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268181
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>

Author: aam

runtime/observatory/tests/service/developer_server_launch_test.dart

@@ -0,0 +1,23 @@
+// Copyright (c) 2022, 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.
+//
+// This test verifies that concurrent requests to start service server still
+// result only in one server being brought up.
+
+import 'dart:async';
+import 'dart:developer';
+import 'package:expect/expect.dart';
+import 'package:observatory/service_io.dart' as S;
+import 'test_helper.dart';
+
+main() async {
+  for (int i = 0; i < 32; i++) {
+    Service.controlWebServer(enable: true, silenceOutput: true);
+  }
+  // Give some time for control messages to arrive to vmservice isolate.
+  await Future.delayed(Duration(seconds: 2));
+  // If the program doesn hang on shutdown, the test passes.
+  // Program hanging means that more than one http server was launched,
+  // but only one gets closed.
+}

runtime/observatory_2/tests/service_2/developer_server_launch_test.dart

@@ -0,0 +1,23 @@
+// Copyright (c) 2022, 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.
+//
+// This test verifies that concurrent requests to start service server still
+// result only in one server being brought up.
+
+import 'dart:async';
+import 'dart:developer';
+import 'package:expect/expect.dart';
+import 'package:observatory/service_io.dart' as S;
+import 'test_helper.dart';
+
+main() async {
+  for (int i = 0; i < 32; i++) {
+    Service.controlWebServer(enable: true, silenceOutput: true);
+  }
+  // Give some time for control messages to arrive to vmservice isolate.
+  await Future.delayed(Duration(seconds: 2));
+  // If the program doesn hang on shutdown, the test passes.
+  // Program hanging means that more than one http server was launched,
+  // but only one gets closed.
+}

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

@@ -150,6 +150,9 @@ class Server {
   bool get running => _server != null;
   bool acceptNewWebSocketConnections = true;
   int _port = -1;
+  // Ensures only one server is started even if many requests to launch
+  // the server come in concurrently.
+  Completer<bool>? _startingCompleter;
 
   /// Returns the server address including the auth token.
   Uri? get serverAddress {
@@ -401,6 +404,19 @@ class Server {
       // Already running.
       return this;
     }
+
+    {
+      final startingCompleter = _startingCompleter;
+      if (startingCompleter != null) {
+        if (!startingCompleter.isCompleted) {
+          await startingCompleter.future;
+        }
+        return this;
+      }
+    }
+
+    final startingCompleter = Completer<bool>();
+    _startingCompleter = startingCompleter;
     // Startup HTTP server.
     Future<bool> startServer() async {
       try {
@@ -431,11 +447,13 @@ class Server {
     }
 
     if (!(await startServer())) {
+      startingCompleter.complete(true);
       return this;
     }
     if (_service.isExiting) {
       serverPrint('Dart VM service HTTP server exiting before listening as '
           'vm service has received exit request\n');
+      startingCompleter.complete(true);
       await shutdown(true);
       return this;
     }
@@ -447,6 +465,7 @@ class Server {
     // Server is up and running.
     _notifyServerState(serverAddress.toString());
     onServerAddressChange('$serverAddress');
+    startingCompleter.complete(true);
     return this;
   }
 
@@ -481,7 +500,14 @@ class Server {
     return serverLocal.close(force: force);
   }
 
-  Future<Server> shutdown(bool forced) {
+  Future<Server> shutdown(bool forced) async {
+    // If start is pending, wait for it to complete.
+    if (_startingCompleter != null) {
+      if (!_startingCompleter!.isCompleted) {
+        await _startingCompleter!.future;
+      }
+    }
+
     if (_server == null) {
       // Not started.
       return Future.value(this);
@@ -492,11 +518,13 @@ class Server {
     return cleanup(forced).then((_) {
       serverPrint('Dart VM service no longer listening on $oldServerAddress');
       _server = null;
+      _startingCompleter = null;
       _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');
       _notifyServerState('');
       onServerAddressChange(null);