Avoid hanging if we don't use explicit `exit()` (#2704)

* Close the httpClient explicitly.
* Close the signal watching after the process ran
* Don't close the oauth client explicitly
* Use a subProcess to run from the embedding
diff --git a/lib/src/command.dart b/lib/src/command.dart
index 47f4f05..a6ecae2 100644
--- a/lib/src/command.dart
+++ b/lib/src/command.dart
@@ -170,6 +170,8 @@
 """);
       }
       return _chooseExitCode(error);
+    } finally {
+      httpClient.close();
     }
   }
 
diff --git a/lib/src/command/global.dart b/lib/src/command/global.dart
index 6b3cde6..a7e8898 100644
--- a/lib/src/command/global.dart
+++ b/lib/src/command/global.dart
@@ -17,10 +17,10 @@
   @override
   String get docUrl => 'https://dart.dev/tools/pub/cmd/pub-global';
 
-  GlobalCommand() {
+  GlobalCommand({bool alwaysUseSubprocess = false}) {
     addSubcommand(GlobalActivateCommand());
     addSubcommand(GlobalDeactivateCommand());
     addSubcommand(GlobalListCommand());
-    addSubcommand(GlobalRunCommand());
+    addSubcommand(GlobalRunCommand(alwaysUseSubprocess: alwaysUseSubprocess));
   }
 }
diff --git a/lib/src/command/global_run.dart b/lib/src/command/global_run.dart
index da5a015..165997a 100644
--- a/lib/src/command/global_run.dart
+++ b/lib/src/command/global_run.dart
@@ -25,7 +25,9 @@
   @override
   bool get allowTrailingOptions => false;
 
-  GlobalRunCommand() {
+  final bool alwaysUseSubprocess;
+
+  GlobalRunCommand({this.alwaysUseSubprocess = false}) {
     argParser.addFlag('enable-asserts', help: 'Enable assert statements.');
     argParser.addFlag('checked', abbr: 'c', hide: true);
     argParser.addMultiOption('enable-experiment',
@@ -66,12 +68,16 @@
 
     final vmArgs = vmArgsFromArgResults(argResults);
     final globalEntrypoint = await globals.find(package);
-    final exitCode = await globals.runExecutable(globalEntrypoint,
-        Executable.adaptProgramName(package, executable), args,
-        vmArgs: vmArgs,
-        enableAsserts: argResults['enable-asserts'] || argResults['checked'],
-        recompile: (executable) => log.warningsOnlyUnlessTerminal(
-            () => globalEntrypoint.precompileExecutable(executable)));
+    final exitCode = await globals.runExecutable(
+      globalEntrypoint,
+      Executable.adaptProgramName(package, executable),
+      args,
+      vmArgs: vmArgs,
+      enableAsserts: argResults['enable-asserts'] || argResults['checked'],
+      recompile: (executable) => log.warningsOnlyUnlessTerminal(
+          () => globalEntrypoint.precompileExecutable(executable)),
+      alwaysUseSubprocess: alwaysUseSubprocess,
+    );
     throw ExitWithException(exitCode);
   }
 }
diff --git a/lib/src/command/run.dart b/lib/src/command/run.dart
index b106472..567a761 100644
--- a/lib/src/command/run.dart
+++ b/lib/src/command/run.dart
@@ -28,8 +28,12 @@
   bool get hidden => deprecated;
 
   final bool deprecated;
+  final bool alwaysUseSubprocess;
 
-  RunCommand({this.deprecated = false}) {
+  RunCommand({
+    this.deprecated = false,
+    this.alwaysUseSubprocess = false,
+  }) {
     argParser.addFlag('enable-asserts', help: 'Enable assert statements.');
     argParser.addFlag('checked', abbr: 'c', hide: true);
     argParser.addMultiOption('enable-experiment',
@@ -93,6 +97,7 @@
       recompile: (executable) => log.warningsOnlyUnlessTerminal(
           () => entrypoint.precompileExecutable(executable)),
       vmArgs: vmArgs,
+      alwaysUseSubprocess: alwaysUseSubprocess,
     );
     throw ExitWithException(exitCode);
   }
@@ -136,10 +141,14 @@
 
     throw ExitWithException(
       await runExecutable(
-          entrypoint, Executable(package, 'bin/$command.dart'), args,
-          vmArgs: vmArgs,
-          enableAsserts: argResults['enable-asserts'] || argResults['checked'],
-          recompile: entrypoint.precompileExecutable),
+        entrypoint,
+        Executable(package, 'bin/$command.dart'),
+        args,
+        vmArgs: vmArgs,
+        enableAsserts: argResults['enable-asserts'] || argResults['checked'],
+        recompile: entrypoint.precompileExecutable,
+        alwaysUseSubprocess: alwaysUseSubprocess,
+      ),
     );
   }
 }
diff --git a/lib/src/executable.dart b/lib/src/executable.dart
index dfbdf2c..d178362 100644
--- a/lib/src/executable.dart
+++ b/lib/src/executable.dart
@@ -7,8 +7,8 @@
 import 'dart:isolate';
 
 import 'package:args/args.dart';
+import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
-import 'package:pedantic/pedantic.dart';
 
 import 'entrypoint.dart';
 import 'exceptions.dart';
@@ -55,7 +55,8 @@
     {bool enableAsserts = false,
     String packagesFile,
     Future<void> Function(Executable) recompile,
-    List<String> vmArgs = const []}) async {
+    List<String> vmArgs = const [],
+    @required bool alwaysUseSubprocess}) async {
   final package = executable.package;
   packagesFile ??= entrypoint.packagesFile;
 
@@ -124,8 +125,14 @@
   var packageConfig = p.absolute(packagesFile);
 
   try {
-    return await _runDartProgram(executablePath, args, packageConfig,
-        enableAsserts: enableAsserts, vmArgs: vmArgs);
+    return await _runDartProgram(
+      executablePath,
+      args,
+      packageConfig,
+      enableAsserts: enableAsserts,
+      vmArgs: vmArgs,
+      alwaysUseSubprocess: alwaysUseSubprocess,
+    );
   } on IsolateSpawnException catch (error) {
     if (!useSnapshot ||
         !error.message.contains('Invalid kernel binary format version')) {
@@ -134,8 +141,14 @@
 
     log.fine('Precompiled executable is out of date.');
     await recompile(executable);
-    return _runDartProgram(executablePath, args, packageConfig,
-        enableAsserts: enableAsserts, vmArgs: vmArgs);
+    return await _runDartProgram(
+      executablePath,
+      args,
+      packageConfig,
+      enableAsserts: enableAsserts,
+      vmArgs: vmArgs,
+      alwaysUseSubprocess: alwaysUseSubprocess,
+    );
   }
 }
 
@@ -149,21 +162,26 @@
 /// Passes [vmArgs] to the vm.
 ///
 /// Returns the programs's exit code.
+///
+/// Tries to run the program as an isolate if no special [vmArgs] are given
+/// otherwise starts new vm in a subprocess. If [alwaysUseSubprocess] is `true`
+/// a new process will always be started.
 Future<int> _runDartProgram(
     String path, List<String> args, String packageConfig,
-    {bool enableAsserts, List<String> vmArgs}) async {
+    {bool enableAsserts,
+    List<String> vmArgs,
+    @required bool alwaysUseSubprocess}) async {
   path = p.absolute(path);
   packageConfig = p.absolute(packageConfig);
 
   // We use Isolate.spawnUri when there are no extra vm-options.
   // That provides better signal handling, and possibly faster startup.
-  if (vmArgs.isEmpty) {
+  if ((!alwaysUseSubprocess) && vmArgs.isEmpty) {
     var argList = args.toList();
-    await isolate.runUri(p.toUri(path), argList, null,
+    return await isolate.runUri(p.toUri(path), argList, null,
         enableAsserts: enableAsserts,
         automaticPackageResolution: packageConfig == null,
         packageConfig: p.toUri(packageConfig));
-    return exitCode;
   } else {
     // By ignoring sigint, only the child process will get it when
     // they are sent to the current process group. That is what happens when
@@ -184,8 +202,7 @@
     // TODO(sigurdm) To handle signals better we would ideally have `exec`
     // semantics without `fork` for starting the subprocess.
     // https://github.com/dart-lang/sdk/issues/41966.
-    unawaited(ProcessSignal.sigint.watch().drain());
-
+    final subscription = ProcessSignal.sigint.watch().listen((e) {});
     final process = await Process.start(
       Platform.resolvedExecutable,
       [
@@ -198,7 +215,9 @@
       mode: ProcessStartMode.inheritStdio,
     );
 
-    return process.exitCode;
+    final exitCode = await process.exitCode;
+    await subscription.cancel();
+    return exitCode;
   }
 }
 
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart
index f96af90..a140721 100644
--- a/lib/src/global_packages.dart
+++ b/lib/src/global_packages.dart
@@ -375,13 +375,21 @@
       {bool enableAsserts = false,
       String packagesFile,
       Future<void> Function(exec.Executable) recompile,
-      List<String> vmArgs = const []}) async {
-    return await exec.runExecutable(entrypoint, executable, args,
-        enableAsserts: enableAsserts,
-        packagesFile: packagesFile, recompile: (exectuable) async {
-      await recompile(exectuable);
-      _refreshBinStubs(entrypoint, executable);
-    }, vmArgs: vmArgs);
+      List<String> vmArgs = const [],
+      @required bool alwaysUseSubprocess}) async {
+    return await exec.runExecutable(
+      entrypoint,
+      executable,
+      args,
+      enableAsserts: enableAsserts,
+      packagesFile: packagesFile,
+      recompile: (exectuable) async {
+        await recompile(exectuable);
+        _refreshBinStubs(entrypoint, executable);
+      },
+      vmArgs: vmArgs,
+      alwaysUseSubprocess: alwaysUseSubprocess,
+    );
   }
 
   /// Gets the path to the lock file for an activated cached package with
diff --git a/lib/src/http.dart b/lib/src/http.dart
index 0aa43bf..e84467b 100644
--- a/lib/src/http.dart
+++ b/lib/src/http.dart
@@ -142,6 +142,9 @@
       return '$name: $value';
     }
   }
+
+  @override
+  void close() => _inner.close();
 }
 
 /// The [_PubHttpClient] wrapped by [httpClient].
@@ -220,6 +223,9 @@
 
     throw PubHttpException(await http.Response.fromStream(streamedResponse));
   }
+
+  @override
+  void close() => _inner.close();
 }
 
 /// The HTTP client to use for all HTTP requests.
diff --git a/lib/src/isolate.dart b/lib/src/isolate.dart
index 9d0a7aa..58169c7 100644
--- a/lib/src/isolate.dart
+++ b/lib/src/isolate.dart
@@ -14,7 +14,7 @@
 /// the [exitCode] variable is set to 255.
 ///
 /// If [buffered] is `true`, this uses [spawnBufferedUri] to spawn the isolate.
-Future runUri(Uri url, List<String> args, Object message,
+Future<int> runUri(Uri url, List<String> args, Object message,
     {bool buffered = false,
     bool enableAsserts,
     bool automaticPackageResolution = false,
@@ -29,12 +29,16 @@
       onError: errorPort.sendPort,
       onExit: exitPort.sendPort);
 
-  errorPort.listen((list) {
+  final subscription = errorPort.listen((list) {
     stderr.writeln('Unhandled exception:');
     stderr.writeln(list[0]);
     stderr.write(list[1]);
     exitCode = 255;
   });
-
-  await exitPort.first;
+  try {
+    await exitPort.first;
+  } finally {
+    await subscription.cancel();
+  }
+  return exitCode;
 }
diff --git a/lib/src/oauth2.dart b/lib/src/oauth2.dart
index fe0cef1..a3e79c4 100644
--- a/lib/src/oauth2.dart
+++ b/lib/src/oauth2.dart
@@ -84,8 +84,10 @@
   }
 }
 
-/// Asynchronously passes an OAuth2 [Client] to [fn], and closes the client when
-/// the [Future] returned by [fn] completes.
+/// Asynchronously passes an OAuth2 [Client] to [fn].
+///
+/// Does not close the client, since that would close the shared client. It must
+/// be closed elsewhere.
 ///
 /// This takes care of loading and saving the client's credentials, as well as
 /// prompting the user for their authorization. It will also re-authorize and
@@ -93,7 +95,9 @@
 Future<T> withClient<T>(SystemCache cache, Future<T> Function(Client) fn) {
   return _getClient(cache).then((client) {
     return fn(client).whenComplete(() {
-      client.close();
+      // TODO(sigurdm): refactor the http subsystem, so we can close [client]
+      // here.
+
       // Be sure to save the credentials even when an error happens.
       _saveCredentials(cache, client.credentials);
     });
diff --git a/lib/src/pub_embeddable_command.dart b/lib/src/pub_embeddable_command.dart
index 2bc6133..6b147fc 100644
--- a/lib/src/pub_embeddable_command.dart
+++ b/lib/src/pub_embeddable_command.dart
@@ -2,8 +2,8 @@
 // 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 'command.dart' show PubCommand;
 import 'command.dart';
+import 'command.dart' show PubCommand;
 import 'command/add.dart';
 import 'command/build.dart';
 import 'command/cache.dart';
@@ -49,12 +49,12 @@
     addSubcommand(CacheCommand());
     addSubcommand(DepsCommand());
     addSubcommand(DowngradeCommand());
-    addSubcommand(GlobalCommand());
+    addSubcommand(GlobalCommand(alwaysUseSubprocess: true));
     addSubcommand(GetCommand());
     addSubcommand(LishCommand());
     addSubcommand(OutdatedCommand());
     addSubcommand(RemoveCommand());
-    addSubcommand(RunCommand(deprecated: true));
+    addSubcommand(RunCommand(deprecated: true, alwaysUseSubprocess: true));
     addSubcommand(UpgradeCommand());
     addSubcommand(UploaderCommand());
     addSubcommand(LoginCommand());
diff --git a/tool/test-bin/pub_command_runner.dart b/tool/test-bin/pub_command_runner.dart
index 056f105..ec32015 100644
--- a/tool/test-bin/pub_command_runner.dart
+++ b/tool/test-bin/pub_command_runner.dart
@@ -2,9 +2,9 @@
 
 import 'package:args/args.dart';
 import 'package:args/command_runner.dart';
-import 'package:pub/src/pub_embeddable_command.dart';
-import 'package:pub/src/log.dart' as log;
 import 'package:pub/src/exit_codes.dart' as exit_codes;
+import 'package:pub/src/log.dart' as log;
+import 'package:pub/src/pub_embeddable_command.dart';
 
 class Runner extends CommandRunner<int> {
   ArgResults _options;