From a4662281fdf356d9202d2a0a8fb57ed04670b094 Mon Sep 17 00:00:00 2001 From: Michael Crumm Date: Fri, 8 Apr 2022 14:52:07 -0700 Subject: [PATCH 1/2] Ensure onStdinClose only completes for non-tty streams The initial stdin commit 144cd35 inverted the TTY conditional b/w the io/node and io/vm implementations of onStdinClose. The node implementation incorrectly checked for the presence of TTY to complete the stream, while the vm implementation correctly checked for its absence. The commit that landed upstream c7ab426 normalized the incorrect behaviour, which means that sass was still not closing when stdin was closed, unless stdin was a TTY. Unfortunately that created a "worst of both worlds" situation because programs that start sass and then close unexpectedly will still leave zombie sass processes running in the background, and wrapper scripts designed to mitigate this exact problem will stop working because moving the process to the background now incorrectly causes the job to stop. This change ensures we only complete the CancelableOperation onStdinClose for non-tty standard input streams. --- lib/src/io/interface.dart | 8 +++++--- lib/src/io/node.dart | 2 +- lib/src/io/vm.dart | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/src/io/interface.dart b/lib/src/io/interface.dart index f25b6cc3e..8e6b5785c 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -95,10 +95,12 @@ String? getEnvironmentVariable(String name) => throw ''; int get exitCode => throw ''; set exitCode(int value) => throw ''; -/// If stdin is a TTY, returns a [CancelableOperation] that completes once it -/// closes. +/// Returns a [CancelableOperation] that completes when stdin closes. /// -/// Otherwise, returns a [CancelableOperation] that never completes. +/// Note that if stdin is a TTY, the operation never completes. This is to +/// avoid interfering with background job systems where reading from stdin +/// and then moving the process to the background would incorrectly cause +/// the job to stop. /// /// As long as this is uncanceled, it will monopolize stdin so that nothing else /// can read from it. diff --git a/lib/src/io/node.dart b/lib/src/io/node.dart index 97a13b009..f38c2e2f0 100644 --- a/lib/src/io/node.dart +++ b/lib/src/io/node.dart @@ -218,7 +218,7 @@ set exitCode(int code) => process.exitCode = code; CancelableOperation onStdinClose() { var completer = CancelableCompleter(); - if (isStdinTTY == true) { + if (isStdinTTY != true) { process.stdin.on('end', allowInterop(() => completer.complete())); } return completer.operation; diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index 72d74fde4..22ff69f8d 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -91,8 +91,8 @@ DateTime modificationTime(String path) { String? getEnvironmentVariable(String name) => io.Platform.environment[name]; CancelableOperation onStdinClose() => io.stdin.hasTerminal - ? CancelableOperation.fromSubscription(io.stdin.listen(null)) - : CancelableCompleter().operation; + ? CancelableCompleter().operation + : CancelableOperation.fromSubscription(io.stdin.listen(null)); Future> watchDir(String path, {bool poll = false}) async { var watcher = poll ? PollingDirectoryWatcher(path) : DirectoryWatcher(path); From 9db2a0b84f1912e3c12ac1f82b19c41da3b14795 Mon Sep 17 00:00:00 2001 From: Michael Crumm Date: Mon, 18 Apr 2022 14:44:18 -0700 Subject: [PATCH 2/2] Add changelog entry --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 446259fec..9322efe77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 1.50.1 + +### Command Line Interface + +* Fix a bug where moving the `--watch` command to the background would + unexpectedly cause the command to stop running unless the standard input + stream was a TTY. + ## 1.50.0 * `@extend` now treats [`:where()`] the same as `:is()`.