[ package:vm_service ] Ensure VmService.onDone doesn't complete until the provided dispose handler has finished

Fixes https://github.com/dart-lang/sdk/issues/43940
TEST=regress_43940_test.dart

Fixed: 43940
Change-Id: I7ad57e0c35a91bf1e1e3dada6662b33b59e2d7bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/176922
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jacob Richman <jacobr@google.com>

pkg/vm_service/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 5.5.1
+- Fix issue where `VmService.onDone` could complete before the provided `DisposeHandler` had finished executing.
+
 ## 5.5.0
 - Update to version `3.42.0` of the spec.
 - Added optional `limit` parameter to `getStack` RPC.

pkg/vm_service/example/vm_service_tester.dart

@@ -128,7 +128,7 @@ void main() {
     print(await serviceClient.resume(isolateRef.id));
 
     print('waiting for client to shut down...');
-    serviceClient.dispose();
+    await serviceClient.dispose();
 
     await serviceClient.onDone;
     print('service client shut down');
@@ -179,7 +179,7 @@ Future testServiceRegistration() async {
   });
   await otherClient.streamListen('Service');
   await completer.future;
-  otherClient.dispose();
+  await otherClient.dispose();
 }
 
 Future testScriptParse(IsolateRef isolateRef) async {

pkg/vm_service/lib/src/vm_service.dart

@@ -1965,8 +1965,8 @@ class VmService implements VmServiceInterface {
 
   Stream<String> get onReceive => _onReceive.stream;
 
-  void dispose() {
-    _streamSub.cancel();
+  Future<void> dispose() async {
+    await _streamSub.cancel();
     _completers.forEach((id, c) {
       final method = _methodCalls[id];
       return c.completeError(RPCError(
@@ -1974,7 +1974,7 @@ class VmService implements VmServiceInterface {
     });
     _completers.clear();
     if (_disposeHandler != null) {
-      _disposeHandler();
+      await _disposeHandler();
     }
     if (!_onDoneCompleter.isCompleted) {
       _onDoneCompleter.complete();
@@ -6772,7 +6772,7 @@ class Timeline extends Response {
   static Timeline parse(Map<String, dynamic> json) =>
       json == null ? null : Timeline._fromJson(json);
 
-  /// A list of timeline events. No order is guaranteed for these events; in
+  /// A list of timeline events. No order is guarenteed for these events; in
   /// particular, these events may be unordered with respect to their
   /// timestamps.
   List<TimelineEvent> traceEvents;

pkg/vm_service/pubspec.yaml

@@ -2,7 +2,7 @@ name: vm_service
 description: >-
   A library to communicate with a service implementing the Dart VM
   service protocol.
-version: 5.5.0
+version: 5.5.1
 
 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/vm_service
 

pkg/vm_service/test/regress_43940_test.dart

@@ -0,0 +1,29 @@
+// Copyright (c) 2020, 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:async';
+
+import 'package:pedantic/pedantic.dart';
+import 'package:test/test.dart';
+import 'package:vm_service/vm_service.dart';
+
+void main() {
+  test('Call dispose handler before onDone completion', () async {
+    final controller = StreamController<String>(onCancel: () async {
+      await Future.delayed(const Duration(seconds: 1));
+    });
+    bool completed = false;
+    final fakeService = VmService(
+      controller.stream,
+      controller.sink.add,
+      disposeHandler: () async {
+        completed = true;
+      },
+    );
+
+    unawaited(fakeService.dispose());
+    await fakeService.onDone;
+    expect(completed, true);
+  });
+}

pkg/vm_service/test/server_test.dart

@@ -420,7 +420,7 @@ void main() {
 
       // Kill the client that registered the handler, it should now fall back
       // on `callServiceExtension`.
-      client.dispose();
+      await client.dispose();
       // This should complete as well.
       await clientConnection.done;
 

pkg/vm_service/tool/dart/generate_dart.dart

@@ -100,8 +100,8 @@ final String _implCode = r'''
 
   Stream<String> get onReceive => _onReceive.stream;
 
-  void dispose() {
-    _streamSub.cancel();
+  Future<void> dispose() async {
+    await _streamSub.cancel();
     _completers.forEach((id, c) {
       final method = _methodCalls[id];
       return c.completeError(RPCError(
@@ -109,7 +109,7 @@ final String _implCode = r'''
     });
     _completers.clear();
     if (_disposeHandler != null) {
-      _disposeHandler();
+      await _disposeHandler();
     }
     if (!_onDoneCompleter.isCompleted) {
       _onDoneCompleter.complete();