[ Service ] Cleanup dart_runtime_service* logging and DDS state

A few minor changes:

 - Log output is now written to stderr.
 - `Service.controlWebServer`'s `silenceOutput` parameter is now
   respected, along with the `SILENT_OBSERVATORY`, `SILENT_VM_SERVICE`,
   and `SILENT_SERVICE` Dart defines.
 - DDS launcher state is cleaned up if the service server is shutdown
   via SIGQUIT.

Change-Id: I0e3e271905ad6bc111151bac086fe7661c23d578
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491280
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/dart_runtime_service/lib/src/dart_development_service_manager.dart

@@ -21,6 +21,7 @@ final class DartDevelopmentServiceManager {
   DartDevelopmentServiceManager({
     required this.frontend,
     required this.launchOnStart,
+    required this.printDtd,
     required this.host,
     required this.port,
   });
@@ -31,6 +32,10 @@ final class DartDevelopmentServiceManager {
   /// is initialized.
   final bool launchOnStart;
 
+  /// `true` if the URI for the DTD instance associated with DDS should be
+  /// made available.
+  final bool printDtd;
+
   /// The host DDS should attempt to bind to.
   final String host;
 
@@ -44,6 +49,17 @@ final class DartDevelopmentServiceManager {
   /// If DDS is not running, [uri] returns null.
   Uri? get uri => _launcher?.uri;
 
+  /// The HTTP [Uri] of the hosted DevTools instance.
+  ///
+  /// Returns `null` if DevTools is not running.
+  Uri? get devToolsUri => _launcher?.devToolsUri;
+
+  /// The [Uri] of the Dart Tooling Daemon instance that is hosted by DevTools.
+  ///
+  /// This will be null if DTD was not started by the DevTools server. For
+  /// example, it may have been started by an IDE.
+  Uri? get dtdUri => printDtd ? _launcher?.dtdUri : null;
+
   final _logger = Logger('$DartDevelopmentServiceManager');
   DartDevelopmentServiceLauncher? _launcher;
 

pkg/dart_runtime_service/lib/src/dart_runtime_service.dart

@@ -30,9 +30,22 @@ class DartRuntimeService {
     required DartRuntimeServiceBackendBuilder backendBuilder,
   }) : authCode = config.disableAuthCodes ? null : generateSecret() {
     if (config.enableLogging) {
-      _logger.onRecord.listen(stdout.writeln);
+      _logger.onRecord.listen(stderr.writeln);
     }
     backend = backendBuilder(this);
+    // We can't use the const constructors here as they won't pickup Dart
+    // environment variables specified at runtime when the service is compiled
+    // to a snapshot. This behavior is somewhat undefined and doesn't work
+    // in AOT, but is maintained for backwards compatibility.
+    silenceServiceOutput =
+        // TODO(48602): deprecate SILENT_OBSERVATORY in favor of
+        // SILENT_VM_SERVICE
+        // ignore: prefer_const_constructors
+        bool.fromEnvironment('SILENT_OBSERVATORY') ||
+        // ignore: prefer_const_constructors
+        bool.fromEnvironment('SILENT_VM_SERVICE') ||
+        // ignore: prefer_const_constructors
+        bool.fromEnvironment('SILENT_SERVICE');
   }
 
   static Future<DartRuntimeService> initialize({
@@ -74,7 +87,14 @@ class DartRuntimeService {
   ///
   /// It's possible that the returned [Uri] is no longer valid if the server
   /// was recently shut down.
-  Uri get httpUri => uri.replace(scheme: 'http');
+  Uri get httpUri => uri.replace(
+    scheme: 'http',
+    pathSegments: [
+      ...uri.pathSegments,
+      // Adds a trailing '/' for backwards compatibility.
+      '',
+    ],
+  );
 
   /// The sse:// URI pointing to this [DartRuntimeService]'s server.
   ///
@@ -127,6 +147,20 @@ class DartRuntimeService {
   /// Returns true if the HTTP server is active.
   bool get isServerRunning => _server != null;
 
+  /// If true, the service won't write any messages to STDOUT.
+  ///
+  /// Note: this does not impact logging output when
+  /// [DartRuntimeServiceOptions.enableLogging] is true.
+  bool silenceServiceOutput = false;
+
+  /// Writes [message] to STDOUT, unless [silenceServiceOutput] is true.
+  void printServiceOutput(String message) {
+    if (silenceServiceOutput) {
+      return;
+    }
+    stdout.writeln(message);
+  }
+
   /// Initializes the service's state without starting the web server.
   Future<void> _initialize() async {
     await backend.initialize();
@@ -154,11 +188,16 @@ class DartRuntimeService {
   ///
   /// This is called when `dart:developer`'s [Service.controlWebServer] is
   /// invoked.
-  // TODO(bkonyi): respect silenceOutput
   Future<void> serverControl({
     required bool enable,
     bool? silenceOutput,
   }) async {
+    if (silenceOutput != null) {
+      _logger.info(
+        'silenceServiceOutput: $silenceServiceOutput -> $silenceOutput',
+      );
+      silenceServiceOutput = silenceOutput;
+    }
     // TODO(bkonyi): verify there's no race conditions
     if (!enable && isServerRunning) {
       await _shutdownServer();
@@ -256,6 +295,7 @@ class DartRuntimeService {
     _server = null;
     _uri = null;
     await server.close();
+    await backend.onServerShutdown();
   }
 
   /// Send a [StreamEvent] to subscribed clients.

pkg/dart_runtime_service/lib/src/dart_runtime_service_backend.dart

@@ -73,6 +73,10 @@ abstract class DartRuntimeServiceBackend<IM extends IsolateManager> {
   /// started.
   Future<void> onServerStarted({required Uri httpUri, required Uri wsUri});
 
+  /// Invoked by the [DartRuntimeService] when the service's HTTP server has
+  /// shutdown.
+  Future<void> onServerShutdown();
+
   /// Invoked when [EventStreamManager.streamListen] is called and the first
   /// client has subscribed to [streamId].
   ///

pkg/dart_runtime_service/test/utils/mocks.dart

@@ -34,6 +34,9 @@ base class FakeDartRuntimeServiceBackend extends Fake
     required Uri wsUri,
   }) async {}
 
+  @override
+  Future<void> onServerShutdown() async {}
+
   @override
   UnmodifiableListView<ServiceRpcHandler> get rpcs =>
       UnmodifiableListView(const []);

pkg/dart_runtime_service_vm/bin/vm_service_entrypoint.dart

@@ -88,7 +88,6 @@ bool _enableServicePortFallback = false;
 bool _waitForDdsToAdvertiseService = false;
 
 @entrypoint
-// ignore: unused_element
 bool _printDtd = false;
 
 // ignore: unused_element
@@ -128,6 +127,7 @@ Future<void> main([List<String> args = const []]) async {
       ddsManager: DartDevelopmentServiceManager(
         frontend: frontend,
         launchOnStart: _waitForDdsToAdvertiseService,
+        printDtd: _printDtd,
         host: _ddsIP,
         port: _ddsPort,
       ),

pkg/dart_runtime_service_vm/lib/dart_runtime_service_vm.dart

@@ -164,10 +164,29 @@ class DartRuntimeServiceVMBackend
       await _ddsManager.start(vmServiceUri: httpUri);
       httpUri = await _ddsManager.ddsConnected;
     }
-    stdout.writeln('The Dart VM service is listening on $httpUri/');
+    frontend.printServiceOutput('The Dart VM service is listening on $httpUri');
+    final devToolsUri = _ddsManager.devToolsUri;
+    if (devToolsUri != null) {
+      frontend.printServiceOutput(
+        'The Dart DevTools debugger and profiler is available at: $devToolsUri',
+      );
+    }
+    final dtdUri = _ddsManager.dtdUri;
+    if (dtdUri != null) {
+      frontend.printServiceOutput(
+        'The Dart Tooling Daemon (DTD) is available at: $dtdUri',
+      );
+    }
     _nativeBindings.onServerAddressChange(httpUri.toString());
   }
 
+  @override
+  Future<void> onServerShutdown() async {
+    // Cleanup DDS state so it can be reinitialized if the server is started
+    // again.
+    await _ddsManager.shutdown();
+  }
+
   @override
   bool onStreamListen({
     required String streamId,
@@ -255,7 +274,9 @@ class DartRuntimeServiceVMBackend
         // isolate.
         _isolateControlMessageHandler(opcode, portId, sendPort, name);
       default:
-        print('Internal vm-service error: ignoring illegal message: $message');
+        _logger.warning(
+          'Internal vm-service error: ignoring illegal message: $message',
+        );
     }
   }