tcgetattr / tcsetattr seemingly behaving weirdly when piping output from VM into less and quitting

[jensjoha]

On my machine at least, doing this:
out/ReleaseX64/dart pkg/kernel/bin/dump.dart out/ReleaseX64/vm_platform_strong.dill 2>stderrstuff | less
and immediately (as soon as less actually appears) typing q to quit less, my terminal becomes unresponsive (i.e. it doesn't echo what I type).

Applying a patch like this

diff --git a/runtime/bin/console_posix.cc b/runtime/bin/console_posix.cc
index af28e153e6c..4677525632c 100644
--- a/runtime/bin/console_posix.cc
+++ b/runtime/bin/console_posix.cc
@@ -53,6 +53,7 @@ class PosixConsole {
     if (status != 0) {
       return;
     }
+    fprintf(stderr, "Saving %d\n", term.c_lflag);
     *flag = term.c_lflag;
   }
 
@@ -63,9 +64,11 @@ class PosixConsole {
     struct termios term;
     int status = TEMP_FAILURE_RETRY(tcgetattr(fd, &term));
     if (status != 0) {
+      fprintf(stderr, "Setting restore mode (3) (%ld) failed with status %d!\n", fd, status);
       return;
     }
     term.c_lflag = flag;
+    fprintf(stderr, "Setting restore mode (3) (%ld) (%d)!\n", fd, flag);
     VOID_TEMP_FAILURE_RETRY(tcsetattr(fd, TCSANOW, &term));
   }

yields

Saving 35329
Setting restore mode (3) (0) (35329)!

in the file stderrstuff.

If I instead just had run out/ReleaseX64/dart hello.dart the output is

Saving 35387
Saving 35387
Saving 35387
Hello, World!
Setting restore mode (3) (1) (35387)!
Setting restore mode (3) (2) (35387)!
Setting restore mode (3) (0) (35387)!

(according to calls to stty -a in both cases, it seems to be ICANON, ECHO, ECHOE and ECHOK missing in the case where the terminal becomes unresponsive).

If I remove the call to tcsetattr the problem goes way (though this isn't a real solution).

So it seems that tcgetattr gives a "wrong" result somehow.

Interestingly enough, if I go to the end of the less input (G) before quitting (q) the problem goes away too, despite that the content of stderrstuff still being the same.

My guess as to what happens is this:

• less sets -ICANON, -ECHO, -ECHOE, -ECHOK before dart calls tcgetattr
• If less goes to the end (and the dart process shuts down (it doesn't appear to shut down before less goes to the end)) before quitting less, dart sets the wrong stuff, but less (once it quit) sets the right stuff. Result in this case: Everything is fine.
• If less is quit before going to the end, less quits and sets the right stuff before dart quits and sets the wrong stuff. Result in this case: Terminal becomes unresponsive.
  So, said another way, my guess is that this is basically a kind of race condition between dart and less :/

/cc @mraleph any ideas or ideas of who to ping on this?

[jensjoha]

Oh, and as a fun little extra bit of information, the problems goes away in debug builds of dart because dart instead dies with an assertion error:

$ out/DebugX64/dart pkg/kernel/bin/dump.dart out/ReleaseX64/vm_platform_strong.dill | less
../../runtime/bin/builtin_natives.cc: 94: error: expected: res == length
version=2.2.1-edge.d320f4254282eb7699ae87f2c5af5d5deebe16ce (Tue Apr 2 12:11:58 2019 +0000) on "linux_x64"
thread=190811, isolate=main(0x55ec6c6b8900)
  [0x000055ec6a2c41a6] dart::Profiler::DumpStackTrace(bool)
  [0x000055ec6a2c41a6] dart::Profiler::DumpStackTrace(bool)
  [0x000055ec6a2c40e4] dart::Profiler::DumpStackTrace(void*)
  [0x000055ec6a600f25] Dart_DumpNativeStackTrace
  [0x000055ec6a6059a3] dart::Assert::Fail(char const*, ...)
  [0x000055ec69ea92ff] dart::bin::Builtin_Builtin_PrintString(_Dart_NativeArguments*)
  [0x000055ec6a2035f9] dart::NativeEntry::AutoScopeNativeCallWrapperNoStackCheck(_Dart_NativeArguments*, void (*)(_Dart_NativeArguments*))
  [0x000055ec6a2034c9] dart::NativeEntry::AutoScopeNativeCallWrapper(_Dart_NativeArguments*, void (*)(_Dart_NativeArguments*))
  [0x00007fef69041339] Unknown symbol
  [0x00007fef620ffdaa] Unknown symbol
  [0x00007fef620ffcff] Unknown symbol
  [0x00007fef620ffc00] Unknown symbol
  [0x00007fef620ff8e9] Unknown symbol
  [0x00007fef620ff753] Unknown symbol
  [0x00007fef622c617a] Unknown symbol
  [0x00007fef676ee51d] Unknown symbol
  [0x00007fef676edfcc] Unknown symbol
  [0x00007fef676ca45c] Unknown symbol
  [0x00007fef676ed146] Unknown symbol
  [0x00007fef676ca45c] Unknown symbol
  [0x00007fef676ece33] Unknown symbol
  [0x00007fef6904153a] Unknown symbol
  [0x000055ec6a15de57] dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&, dart::Array const&, unsigned long)
  [0x000055ec6a15d8d1] dart::DartEntry::InvokeFunction(dart::Function const&, dart::Array const&)
  [0x000055ec6a160bd2] dart::DartLibraryCalls::HandleMessage(dart::Object const&, dart::Instance const&)
  [0x000055ec6a1b5ad3] dart::IsolateMessageHandler::HandleMessage(dart::Message*)
  [0x000055ec6a1fee69] dart::MessageHandler::HandleMessages(dart::MonitorLocker*, bool, bool)
  [0x000055ec6a1ffa0b] dart::MessageHandler::TaskCallback()
  [0x000055ec6a200a83] Unknown symbol
  [0x000055ec6a3916d3] dart::ThreadPool::Worker::Loop()
  [0x000055ec6a39147d] dart::ThreadPool::Worker::Main(unsigned long)
  [0x000055ec6a2bbca6] Unknown symbol
-- End of DumpStackTrace

[mraleph]

I think these parts are largely unowned :) @sortie owns dart:io now - so he might be able to help you with this.

[sortie]

Yeah, I've hit this super annoying behavior a number of times. Btw, if it happens, you can run stty sane in your terminal to restore its functionality. You won't see yourself typing it, but the terminal will work properly after you press enter (you'll want to press enter yet again, or clear the screen, to make the shell prompt fully consistent). You correctly notice a workaround, if the dart process finishes its output (by going to the end of the output in less), it will restore the same terminal settings that were in effect, and less will restore the original ones correctly.

The problem is that the Dart VM is unconditionally saving and restoring the terminal settings for stdin, stdout, and stderr; regardless of whether the Dart program even used them as terminals. There are some conditions where we probably want to restore the terminal settings, so we'll need to identify which ones they are, and make sure it only happens under those circumstances.

I'll be happy to take a look at this. Although I won't have the opportunity until I return from my vacation.