Console.Unix: reset terminal at exit in less cases to avoid blocking the parent....
authorTom Deseyn <tom.deseyn@gmail.com>
Wed, 7 Oct 2020 02:25:43 +0000 (04:25 +0200)
committerGitHub <noreply@github.com>
Wed, 7 Oct 2020 02:25:43 +0000 (22:25 -0400)
* Console.Unix: reset terminal at exit in less cases to avoid blocking the parent.

When a parent is fetching the Console.Cursor position, it configures the terminal
to not echo, writes an escape sequence to query the position, and then reads the
position from stdin.

Because this doesn't happen atomically a child process can overwrite the terminal
settings to echo before the parent starts reading. This causes the position to
be echoed to the user, and the parent gets stuck waiting for input on stdin.

Currently terminal settings are reset at exit for applications that use the
Console or Process class. This change tracks whether the application has
changes the terminal settings and only then resets the terminal settings.

This doesn't fix the issue, but makes it less likely to occur.

* TermInfo changes are no longer needed

* Only configure terminal for child if we've touched the settings.

* minor change to comment wording

src/libraries/Native/Unix/System.Native/pal_console.c

index 9a9dfab..c18e3f8 100644 (file)
@@ -91,6 +91,7 @@ static struct termios g_currentTermios;       // the latest attributes set
 static bool g_reading = false;                // tracks whether the application is performing a Console.Read operation
 static bool g_childUsesTerminal = false;      // tracks whether a child process is using the terminal
 static bool g_terminalUninitialized = false;  // tracks whether the application is terminating
+static bool g_terminalConfigured = false;     // tracks whether the application configured the terminal.
 
 static bool g_hasTty = false;                  // cache we are not a tty
 
@@ -153,6 +154,7 @@ static bool TcSetAttr(struct termios* termios, bool blockIfBackground)
     // On success, update the cached value.
     if (rv)
     {
+        g_terminalConfigured = true;
         g_hasCurrentTermios = true;
         g_currentTermios = *termios;
     }
@@ -212,7 +214,11 @@ void UninitializeTerminal()
     {
         if (!g_terminalUninitialized)
         {
-            TcSetAttr(&g_initTermios, /* blockIfBackground */ false);
+            // Avoid configuring the terminal: only reset terminal settings when our process has changed them.
+            if (g_terminalConfigured)
+            {
+                TcSetAttr(&g_initTermios, /* blockIfBackground */ false);
+            }
 
             g_terminalUninitialized = true;
         }
@@ -262,7 +268,11 @@ void SystemNative_ConfigureTerminalForChildProcess(int32_t childUsesTerminal)
             g_hasCurrentTermios = false;
         }
 
-        ConfigureTerminal(g_signalForBreak, /* forChild */ childUsesTerminal, /* minChars */ 1, /* decisecondsTimeout */ 0, /* blockIfBackground */ false);
+        // Avoid configuring the terminal: only change terminal settings when our process has changed them.
+        if (g_terminalConfigured)
+        {
+            ConfigureTerminal(g_signalForBreak, /* forChild */ childUsesTerminal, /* minChars */ 1, /* decisecondsTimeout */ 0, /* blockIfBackground */ false);
+        }
 
         // Redo "Application mode" when there are no more children using the terminal.
         if (!childUsesTerminal)