[io] Don't restore terminal state on exit.

This is breaking change #45630.

The Dart VM has until now restored the terminal settings upon exit to
their initial values for stdin, stdout, and stderr. This change removes
that automatic behavior in favor of having the program do the
restoration. Previously the intention was that dart programs can
enable/disable echoing and line buffering and not worry about restoring
the original settings.

However, the VM doing so unconditionally leads to undesirable behavior
e.g. when the program does not care about terminal settings and is
sharing a process group with a program that does care. E.g. if dart's
output is piped into less(1), then there is a race condition where dart
might see the raw terminal settings set by less(1), and if the dart VM
exits after less(1) has exited, then it will restore the raw terminal
settings, leaving the user with a seemingly defective shell with echo
disabled. This race condition can be reproduced using:

    cat > yes.dart << EOF
    main() {
      for (int i = 0; i < 1000000; i++) {
        print("yes");
      }
    }
    EOF
    stty; (sleep 1 && dart yes.dart) | less; stty; stty sane

The user will end up with a shell with echo behavior disabled. The stty
command shows the current terminal settings, where the difference can be
seen, and 'stty sane' fixes the settings before returning to the shell
prompt. The 'stty sane' call can be omitted to see the defective shell
prompt.

This change removes the terminal restoring behavior (added in Dart
2.0.0) and instead asks applications to do the restoration themselves.
The new design matches how programs in other programming languages
implement interactive input that changes terminal settings.

Furthermore the `echoMode` setting now only controls the `echo` local
mode and no longer sets the `echonl` local mode on POSIX systems (which
controls whether newline are echoed even if the regular echo mode is
disabled). The `echonl` local mode is usually turned off in common shell
environments. Programs that wish to control the `echonl` local mode can
use the new `echoNewlineMode` setting. This change is required to
prevent the reoccurence of #30318 when programs manually restore
`echoMode`.

Windows has further considerations: It also saves the console code pages
and restore them if they were not UTF-8. This behavior is retained as it
is useful and needed for Dart's output to function properly. ANSI output
sequences are also turned on via ENABLE_VIRTUAL_TERMINAL_PROCESSING,
which is slightly changed in this change to only rsetore that setting if
it wasn't already on for consistency.

Closes #36453
Closes #45630

TEST=Reproduced with less as above

Change-Id: I2991f9c7f47b97fe475c1ad6edeb769024f8d0db
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/190484
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Jonas Termansen <sortie@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>

Author: sortie

CHANGELOG.md

@@ -54,6 +54,52 @@ them, you must set the lower bound on the SDK constraint for your package to
 - Add `connectionState` attribute and `connectionstatechange` listener to
   `RtcPeerConnection`.
 
+#### `dart:io`
+
+- **Breaking Change** [#45630][]: The Dart VM no longer automatically restores
+    the initial terminal settings upon exit. Programs that change the `Stdin`
+    settings `lineMode` and `echoMode` are now responsible for restoring the
+    settings upon program exit. E.g. a program disabling `echoMode` will now
+    need to restore the setting itself and handle exiting by the appropriate
+    signals if desired:
+
+    ```dart
+    import 'dart:io';
+    import 'dart:async';
+
+    main() {
+      bool echoWasEnabled = stdin.echoMode;
+      try {
+        late StreamSubscription subscription;
+        subscription = ProcessSignal.sigint.watch().listen((ProcessSignal signal) {
+          stdin.echoMode = echoWasEnabled;
+          subscription.cancel();
+          Process.killPid(pid, signal); /* Die by the signal. */
+        });
+        stdin.echoMode = false;
+      } finally {
+        stdin.echoMode = echoWasEnabled;
+      }
+    }
+    ```
+
+    This change is needed to fix [#36453][] where the dart programs not caring
+    about the terminal settings can inadverently corrupt the terminal settings
+    when e.g. piping into less.
+
+    Furthermore the `echoMode` setting now only controls the `echo` local mode
+    and no longer sets the `echonl` local mode on POSIX systems (which controls
+    whether newline are echoed even if the regular echo mode is disabled). The
+    `echonl` local mode is usually turned off in common shell environments.
+    Programs that wish to control the `echonl` local mode can use the new
+    `echoNewlineMode` setting.
+
+    The Windows console code pages (if not UTF-8) and ANSI escape code support
+    (if disabled) remain restored when the VM exits.
+
+[#45630]: https://github.com/dart-lang/sdk/issues/45630
+[#36453]: https://github.com/dart-lang/sdk/issues/36453
+
 #### `dart:js_util`
 
 - Added `dartify` and a number of minor helper functions.

runtime/bin/console_posix.cc

@@ -18,71 +18,10 @@
 namespace dart {
 namespace bin {
 
-class PosixConsole {
- public:
-  static const tcflag_t kInvalidFlag = -1;
-
-  static void Initialize() {
-    SaveMode(STDOUT_FILENO, &stdout_initial_c_lflag_);
-    SaveMode(STDERR_FILENO, &stderr_initial_c_lflag_);
-    SaveMode(STDIN_FILENO, &stdin_initial_c_lflag_);
-  }
-
-  static void Cleanup() {
-    RestoreMode(STDOUT_FILENO, stdout_initial_c_lflag_);
-    RestoreMode(STDERR_FILENO, stderr_initial_c_lflag_);
-    RestoreMode(STDIN_FILENO, stdin_initial_c_lflag_);
-    ClearLFlags();
-  }
-
- private:
-  static tcflag_t stdout_initial_c_lflag_;
-  static tcflag_t stderr_initial_c_lflag_;
-  static tcflag_t stdin_initial_c_lflag_;
-
-  static void ClearLFlags() {
-    stdout_initial_c_lflag_ = kInvalidFlag;
-    stderr_initial_c_lflag_ = kInvalidFlag;
-    stdin_initial_c_lflag_ = kInvalidFlag;
-  }
-
-  static void SaveMode(intptr_t fd, tcflag_t* flag) {
-    ASSERT(flag != NULL);
-    struct termios term;
-    int status = TEMP_FAILURE_RETRY(tcgetattr(fd, &term));
-    if (status != 0) {
-      return;
-    }
-    *flag = term.c_lflag;
-  }
-
-  static void RestoreMode(intptr_t fd, tcflag_t flag) {
-    if (flag == kInvalidFlag) {
-      return;
-    }
-    struct termios term;
-    int status = TEMP_FAILURE_RETRY(tcgetattr(fd, &term));
-    if (status != 0) {
-      return;
-    }
-    term.c_lflag = flag;
-    VOID_TEMP_FAILURE_RETRY(tcsetattr(fd, TCSANOW, &term));
-  }
-
-  DISALLOW_ALLOCATION();
-  DISALLOW_IMPLICIT_CONSTRUCTORS(PosixConsole);
-};
-
-tcflag_t PosixConsole::stdout_initial_c_lflag_ = PosixConsole::kInvalidFlag;
-tcflag_t PosixConsole::stderr_initial_c_lflag_ = PosixConsole::kInvalidFlag;
-tcflag_t PosixConsole::stdin_initial_c_lflag_ = PosixConsole::kInvalidFlag;
-
 void Console::SaveConfig() {
-  PosixConsole::Initialize();
 }
 
 void Console::RestoreConfig() {
-  PosixConsole::Cleanup();
 }
 
 }  // namespace bin

runtime/bin/console_win.cc

@@ -105,6 +105,10 @@ class ConsoleWin {
     /// to reset the state when we cleanup.
     if ((h != INVALID_HANDLE_VALUE) && GetConsoleMode(h, &mode)) {
       old_mode = mode;
+      // No reason to restore the mode on exit if it was already desirable.
+      if ((mode & flags) == flags) {
+        return kInvalidFlag;
+      }
       if (flags != 0) {
         const DWORD request = mode | flags;
         SetConsoleMode(h, request);

runtime/bin/io_natives.cc

@@ -176,6 +176,8 @@ namespace bin {
   V(Stdin_ReadByte, 1)                                                         \
   V(Stdin_GetEchoMode, 1)                                                      \
   V(Stdin_SetEchoMode, 2)                                                      \
+  V(Stdin_GetEchoNewlineMode, 1)                                               \
+  V(Stdin_SetEchoNewlineMode, 2)                                               \
   V(Stdin_GetLineMode, 1)                                                      \
   V(Stdin_SetLineMode, 2)                                                      \
   V(Stdin_AnsiSupported, 1)                                                    \

runtime/bin/stdio.cc

@@ -85,6 +85,39 @@ void FUNCTION_NAME(Stdin_SetEchoMode)(Dart_NativeArguments args) {
   }
 }
 
+void FUNCTION_NAME(Stdin_GetEchoNewlineMode)(Dart_NativeArguments args) {
+  bool enabled = false;
+  intptr_t fd;
+  if (!GetIntptrArgument(args, 0, &fd)) {
+    return;
+  }
+  if (Stdin::GetEchoNewlineMode(fd, &enabled)) {
+    Dart_SetBooleanReturnValue(args, enabled);
+  } else {
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError());
+  }
+}
+
+void FUNCTION_NAME(Stdin_SetEchoNewlineMode)(Dart_NativeArguments args) {
+  intptr_t fd;
+  if (!GetIntptrArgument(args, 0, &fd)) {
+    return;
+  }
+  bool enabled;
+  Dart_Handle status = Dart_GetNativeBooleanArgument(args, 1, &enabled);
+  if (Dart_IsError(status)) {
+    // The caller is expecting an OSError if something goes wrong.
+    OSError os_error(-1, "Invalid argument", OSError::kUnknown);
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError(&os_error));
+    return;
+  }
+  if (Stdin::SetEchoNewlineMode(fd, enabled)) {
+    Dart_SetReturnValue(args, Dart_True());
+  } else {
+    Dart_SetReturnValue(args, DartUtils::NewDartOSError());
+  }
+}
+
 void FUNCTION_NAME(Stdin_GetLineMode)(Dart_NativeArguments args) {
   bool enabled = false;
   intptr_t fd;

runtime/bin/stdio.h

@@ -20,6 +20,9 @@ class Stdin {
   static bool GetEchoMode(intptr_t fd, bool* enabled);
   static bool SetEchoMode(intptr_t fd, bool enabled);
 
+  static bool GetEchoNewlineMode(intptr_t fd, bool* enabled);
+  static bool SetEchoNewlineMode(intptr_t fd, bool enabled);
+
   static bool GetLineMode(intptr_t fd, bool* enabled);
   static bool SetLineMode(intptr_t fd, bool enabled);
 

runtime/bin/stdio_android.cc

@@ -44,9 +44,34 @@ bool Stdin::SetEchoMode(intptr_t fd, bool enabled) {
     return false;
   }
   if (enabled) {
-    term.c_lflag |= (ECHO | ECHONL);
+    term.c_lflag |= ECHO;
   } else {
-    term.c_lflag &= ~(ECHO | ECHONL);
+    term.c_lflag &= ~(ECHO);
+  }
+  status = NO_RETRY_EXPECTED(tcsetattr(fd, TCSANOW, &term));
+  return (status == 0);
+}
+
+bool Stdin::GetEchoNewlineMode(intptr_t fd, bool* enabled) {
+  struct termios term;
+  int status = NO_RETRY_EXPECTED(tcgetattr(fd, &term));
+  if (status != 0) {
+    return false;
+  }
+  *enabled = ((term.c_lflag & ECHONL) != 0);
+  return true;
+}
+
+bool Stdin::SetEchoNewlineMode(intptr_t fd, bool enabled) {
+  struct termios term;
+  int status = NO_RETRY_EXPECTED(tcgetattr(fd, &term));
+  if (status != 0) {
+    return false;
+  }
+  if (enabled) {
+    term.c_lflag |= ECHONL;
+  } else {
+    term.c_lflag &= ~(ECHONL);
   }
   status = NO_RETRY_EXPECTED(tcsetattr(fd, TCSANOW, &term));
   return (status == 0);

runtime/bin/stdio_fuchsia.cc

@@ -34,6 +34,16 @@ bool Stdin::SetEchoMode(intptr_t fd, bool enabled) {
   return false;
 }
 
+bool Stdin::GetEchoNewlineMode(intptr_t fd, bool* enabled) {
+  errno = ENOSYS;
+  return false;
+}
+
+bool Stdin::SetEchoNewlineMode(intptr_t fd, bool enabled) {
+  errno = ENOSYS;
+  return false;
+}
+
 bool Stdin::GetLineMode(intptr_t fd, bool* enabled) {
   errno = ENOSYS;
   return false;

runtime/bin/stdio_linux.cc

@@ -44,9 +44,34 @@ bool Stdin::SetEchoMode(intptr_t fd, bool enabled) {
     return false;
   }
   if (enabled) {
-    term.c_lflag |= (ECHO | ECHONL);
+    term.c_lflag |= ECHO;
   } else {
-    term.c_lflag &= ~(ECHO | ECHONL);
+    term.c_lflag &= ~(ECHO);
+  }
+  status = NO_RETRY_EXPECTED(tcsetattr(fd, TCSANOW, &term));
+  return (status == 0);
+}
+
+bool Stdin::GetEchoNewlineMode(intptr_t fd, bool* enabled) {
+  struct termios term;
+  int status = NO_RETRY_EXPECTED(tcgetattr(fd, &term));
+  if (status != 0) {
+    return false;
+  }
+  *enabled = ((term.c_lflag & ECHONL) != 0);
+  return true;
+}
+
+bool Stdin::SetEchoNewlineMode(intptr_t fd, bool enabled) {
+  struct termios term;
+  int status = NO_RETRY_EXPECTED(tcgetattr(fd, &term));
+  if (status != 0) {
+    return false;
+  }
+  if (enabled) {
+    term.c_lflag |= ECHONL;
+  } else {
+    term.c_lflag &= ~(ECHONL);
   }
   status = NO_RETRY_EXPECTED(tcsetattr(fd, TCSANOW, &term));
   return (status == 0);

runtime/bin/stdio_macos.cc

@@ -44,9 +44,34 @@ bool Stdin::SetEchoMode(intptr_t fd, bool enabled) {
     return false;
   }
   if (enabled) {
-    term.c_lflag |= (ECHO | ECHONL);
+    term.c_lflag |= ECHO;
   } else {
-    term.c_lflag &= ~(ECHO | ECHONL);
+    term.c_lflag &= ~(ECHO);
+  }
+  status = NO_RETRY_EXPECTED(tcsetattr(fd, TCSANOW, &term));
+  return (status == 0);
+}
+
+bool Stdin::GetEchoNewlineMode(intptr_t fd, bool* enabled) {
+  struct termios term;
+  int status = NO_RETRY_EXPECTED(tcgetattr(fd, &term));
+  if (status != 0) {
+    return false;
+  }
+  *enabled = ((term.c_lflag & ECHONL) != 0);
+  return true;
+}
+
+bool Stdin::SetEchoNewlineMode(intptr_t fd, bool enabled) {
+  struct termios term;
+  int status = NO_RETRY_EXPECTED(tcgetattr(fd, &term));
+  if (status != 0) {
+    return false;
+  }
+  if (enabled) {
+    term.c_lflag |= ECHONL;
+  } else {
+    term.c_lflag &= ~(ECHONL);
   }
   status = NO_RETRY_EXPECTED(tcsetattr(fd, TCSANOW, &term));
   return (status == 0);

runtime/bin/stdio_win.cc

@@ -56,6 +56,19 @@ bool Stdin::SetEchoMode(intptr_t fd, bool enabled) {
   return SetConsoleMode(h, mode);
 }
 
+bool Stdin::GetEchoNewlineMode(intptr_t fd, bool* enabled) {
+  *enabled = false;
+  return true;
+}
+
+bool Stdin::SetEchoNewlineMode(intptr_t fd, bool enabled) {
+  if (enabled) {
+    SetLastError(ERROR_NOT_CAPABLE);
+    return false;
+  }
+  return true;
+}
+
 bool Stdin::GetLineMode(intptr_t fd, bool* enabled) {
   HANDLE h = GetStdHandle(STD_INPUT_HANDLE);
   DWORD mode;

sdk/lib/_internal/js_dev_runtime/patch/io_patch.dart

@@ -687,6 +687,16 @@ class Stdin {
     throw UnsupportedError("Stdin.echoMode");
   }
 
+  @patch
+  bool get echoNewlineMode {
+    throw UnsupportedError("Stdin.echoNewlineMode");
+  }
+
+  @patch
+  void set echoNewlineMode(bool enabled) {
+    throw UnsupportedError("Stdin.echoNewlineMode");
+  }
+
   @patch
   bool get lineMode {
     throw UnsupportedError("Stdin.lineMode");

sdk/lib/_internal/js_runtime/lib/io_patch.dart

@@ -687,6 +687,16 @@ class Stdin {
     throw new UnsupportedError("Stdin.echoMode");
   }
 
+  @patch
+  bool get echoNewlineMode {
+    throw UnsupportedError("Stdin.echoNewlineMode");
+  }
+
+  @patch
+  void set echoNewlineMode(bool enabled) {
+    throw UnsupportedError("Stdin.echoNewlineMode");
+  }
+
   @patch
   bool get lineMode {
     throw new UnsupportedError("Stdin.lineMode");