From 294bb5b10419ccf5ce2810f0fe1af7ce771aecf5 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Sun, 31 Mar 2019 01:28:18 +0100 Subject: [PATCH] Console: toggle terminal echo based on presence of interactive child processes (dotnet/corefx#35621) * Console: toggle terminal echo based on presence of interactive child processes .NET applications echo characters during a Console.Read. By default, Unix terminals echo characters as the user is typing them. When a .NET Core application launches an interactive application (e.g. 'vi') that application expects to find the terminal in an echoing state. To make both work, corefx was disabling echo during Console.Read operations, and turning it back on when the read is done. This changes to echoing while there are interactive applications and not echoing when there are none. This means we no longer need to toggle echo off/on for each Console.Read. And the terminal will no longer echo when there is no Console.Read going on. * Make some tcsetattr operations non-blocking * Fix order between changing console settings and SetExited * Fix unbalanced lock/unlocks * Cache notty * Prefer configuring the terminal for Console over child processes * Update comments * Rename some 'console' to 'terminal' * Only decrement when child is using terminal * Rename ConfigureTerminalForConsole back to In/UninitializeConsoleBefore/AfterRead * Move some code to original place * Add some extra comments * PR feedback * typo Commit migrated from https://github.com/dotnet/corefx/commit/007571b90c3cc555cd30f3b467b87c22f9f665e0 --- .../Interop.ConfigureTerminalForChildProcess.cs | 15 + ...Interop.InitializeTerminalAndSignalHandling.cs} | 4 +- .../System.Native/Interop.RegisterForSigChld.cs | 2 +- .../Native/Unix/System.Native/pal_console.c | 326 +++++++++++++++------ .../Native/Unix/System.Native/pal_console.h | 17 +- .../Native/Unix/System.Native/pal_signal.c | 66 ++--- .../Native/Unix/System.Native/pal_signal.h | 9 +- .../System.Console/src/System.Console.csproj | 4 +- .../System.Console/src/System/ConsolePal.Unix.cs | 4 +- .../System.Console/src/System/IO/StdInReader.cs | 3 +- .../src/System.Diagnostics.Process.csproj | 6 + .../src/System/Diagnostics/Process.Unix.cs | 81 ++++- .../System/Diagnostics/ProcessWaitState.Unix.cs | 22 +- 13 files changed, 375 insertions(+), 184 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.ConfigureTerminalForChildProcess.cs rename src/libraries/Common/src/Interop/Unix/System.Native/{Interop.InitializeConsole.cs => Interop.InitializeTerminalAndSignalHandling.cs} (79%) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ConfigureTerminalForChildProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ConfigureTerminalForChildProcess.cs new file mode 100644 index 0000000..75c116d --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ConfigureTerminalForChildProcess.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_ConfigureTerminalForChildProcess")] + internal static extern unsafe void ConfigureTerminalForChildProcess(bool childUsesTerminal); + } +} diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeConsole.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeTerminalAndSignalHandling.cs similarity index 79% rename from src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeConsole.cs rename to src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeTerminalAndSignalHandling.cs index aeb1b69..c6ca698 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeConsole.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.InitializeTerminalAndSignalHandling.cs @@ -8,8 +8,8 @@ internal static partial class Interop { internal static partial class Sys { - [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_InitializeConsole", SetLastError = true)] - internal static extern bool InitializeConsole(); + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_InitializeTerminalAndSignalHandling", SetLastError = true)] + internal static extern bool InitializeTerminalAndSignalHandling(); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetKeypadXmit")] internal static extern void SetKeypadXmit(string terminfoString); diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs index f0d428d..9c374ce 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.RegisterForSigChld.cs @@ -11,6 +11,6 @@ internal partial class Interop internal delegate void SigChldCallback(bool reapAll); [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_RegisterForSigChld")] - internal static extern bool RegisterForSigChld(SigChldCallback handler); + internal static extern void RegisterForSigChld(SigChldCallback handler); } } diff --git a/src/libraries/Native/Unix/System.Native/pal_console.c b/src/libraries/Native/Unix/System.Native/pal_console.c index afbbd8e..0c1250b 100644 --- a/src/libraries/Native/Unix/System.Native/pal_console.c +++ b/src/libraries/Native/Unix/System.Native/pal_console.c @@ -17,6 +17,8 @@ #include #include #include +#include +#include int32_t SystemNative_GetWindowSize(WinSize* windowSize) { @@ -44,7 +46,7 @@ int32_t SystemNative_IsATty(intptr_t fd) static char* g_keypadXmit = NULL; // string used to enable application mode, from terminfo -static void WriteKeypadXmit() // used in a signal handler, must be signal-safe +static void WriteKeypadXmit() { // If a terminfo "application mode" keypad_xmit string has been supplied, // write it out to the terminal to enter the mode. @@ -71,92 +73,205 @@ void SystemNative_SetKeypadXmit(const char* terminfoString) WriteKeypadXmit(); } -static bool g_readInProgress = false; // tracks whether a read is currently in progress, such that attributes have been changed -static bool g_signalForBreak = true; // tracks whether the terminal should send signals for breaks, such that attributes have been changed -static bool g_haveInitTermios = false; // whether g_initTermios has been initialized -static struct termios g_initTermios; // the initial attributes captured when Console was initialized -static struct termios g_preReadTermios; // the original attributes captured before a read; valid if g_readInProgress is true -static struct termios g_currTermios; // the current attributes set during a read; valid if g_readInProgress is true +static pthread_mutex_t g_lock = PTHREAD_MUTEX_INITIALIZER; // prevents races when initializing and changing the terminal. -void UninitializeConsole() +static bool g_signalForBreak = true; // tracks whether the terminal should send signals for breaks, such that attributes have been changed + +static bool g_haveInitTermios = false; // tracks whether g_initTermios has been initialized +static struct termios g_initTermios; // the initial attributes captured + +static bool g_hasCurrentTermios = false; // tracks whether g_currentTermios is valid +static struct termios g_currentTermios; // the latest attributes set + +// The terminal can be used by the .NET application via the Console class. +// It may also be used by child processes that are started via the Process class. +// The terminal needs to be configured differently depending on the user. +// ConfigureTerminalForXXX are called to change the configuration. +// When it is ambiguous whether we should configure for Console/a child Process, +// we prefer configuring for the Console. +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_noTty = false; // cache we are not a tty + +static volatile bool g_receivedSigTtou = false; + +static void ttou_handler(int signo) { - // pal_signal.cpp calls this on SIGQUIT/SIGINT. - // This can happen when SystemNative_InitializeConsole was not called. - - // Put the attributes back to what they were when the console was initially initialized. - // We only do so, however, if we have explicitly modified the termios; doing so always - // can result in problems if the app is in the background, as then attempting to call - // tcsetattr on STDIN_FILENO will suspend the app and prevent its shutdown. We also don't - // want to, for example, just compare g_currTermios with g_initTermios, as we'd then be - // factoring in changes made by other apps or by user code. - if (g_haveInitTermios && // we successfully initialized the console - (g_readInProgress || !g_signalForBreak)) // we modified attributes + (void)signo; + g_receivedSigTtou = true; +} + +static void InstallTTOUHandler(void (*handler)(int), int flags) +{ + struct sigaction action; + memset(&action, 0, sizeof(action)); + action.sa_handler = handler; + action.sa_flags = flags; + int rvSigaction = sigaction(SIGTTOU, &action, NULL); + assert(rvSigaction == 0); + (void)rvSigaction; +} + +static bool TcSetAttr(struct termios* termios, bool blockIfBackground) +{ + if (g_terminalUninitialized) + { + // The application is exiting, we mustn't change terminal settings. + return true; + } + + if (!blockIfBackground) + { + // When the process is running in background, changing terminal settings + // will stop it (default SIGTTOU action). + // We change SIGTTOU's disposition to get EINTR instead. + // This thread may be used to run a signal handler, which may write to + // stdout. We set SA_RESETHAND to avoid that handler's write loops infinitly + // on EINTR when the process is running in background and the terminal + // configured with TOSTOP. + InstallTTOUHandler(ttou_handler, (int)SA_RESETHAND); + + g_receivedSigTtou = false; + } + + bool rv = tcsetattr(STDIN_FILENO, TCSANOW, termios) >= 0; + + if (!blockIfBackground) + { + if (!rv && errno == EINTR && g_receivedSigTtou) + { + // Operation failed because we are background + // pretend it went fine. + rv = true; + } + + // Restore default SIGTTOU handler. + InstallTTOUHandler(SIG_DFL, 0); + } + + // On success, update the cached value. + if (rv) { - tcsetattr(STDIN_FILENO, TCSANOW, &g_initTermios); - // ignore any failure + g_hasCurrentTermios = true; + g_currentTermios = *termios; } + + return rv; } -static void IncorporateBreak(struct termios *termios, int32_t signalForBreak) +static bool ConfigureTerminal(bool signalForBreak, bool forChild, uint8_t minChars, uint8_t decisecondsTimeout, bool blockIfBackground) { - assert(termios != NULL); - assert(signalForBreak == 0 || signalForBreak == 1); + if (g_noTty) + { + errno = ENOTTY; + return false; + } + + g_childUsesTerminal = forChild; + + assert(g_haveInitTermios); + struct termios termios = g_initTermios; if (signalForBreak) - termios->c_lflag |= (uint32_t)ISIG; + termios.c_lflag |= (uint32_t)ISIG; else - termios->c_lflag &= (uint32_t)(~ISIG); -} + termios.c_lflag &= (uint32_t)(~ISIG); -// In order to support Console.ReadKey(intercept: true), we need to disable echo and canonical mode. -// We have two main choices: do so for the entire app, or do so only while in the Console.ReadKey(true). -// The former has a huge downside: the terminal is in a non-echo state, so anything else that runs -// in the same terminal won't echo even if it expects to, e.g. using Process.Start to launch an interactive, -// program, or P/Invoking to a native library that reads from stdin rather than using Console. The second -// also has a downside, in that any typing which occurs prior to invoking Console.ReadKey(true) will -// be visible even though it wasn't supposed to be. The downsides of the former approach are so large -// and the cons of the latter minimal and constrained to the one API that we've chosen the second approach. -// Thus, InitializeConsoleBeforeRead is called to set up the state of the console, then a read is done, -// and then UninitializeConsoleAfterRead is called. -void SystemNative_InitializeConsoleBeforeRead(uint8_t minChars, uint8_t decisecondsTimeout) -{ - struct termios newTermios; - if (tcgetattr(STDIN_FILENO, &newTermios) >= 0) + if (!forChild) + { + termios.c_iflag &= (uint32_t)(~(IXON | IXOFF)); + termios.c_lflag &= (uint32_t)(~(ECHO | ICANON | IEXTEN)); + } + + termios.c_cc[VMIN] = minChars; + termios.c_cc[VTIME] = decisecondsTimeout; + + // Check if the settings have changed. + if (g_hasCurrentTermios) { - if (!g_readInProgress) + if (g_currentTermios.c_lflag == termios.c_lflag && + g_currentTermios.c_iflag == termios.c_iflag && + g_currentTermios.c_cc[VMIN] == termios.c_cc[VMIN] && + g_currentTermios.c_cc[VMIN] == termios.c_cc[VMIN]) { - // Store the original settings, but only if we didn't already. This function - // may be called when the process is resumed after being suspended, and if - // that happens during a read, we'll call this function to reset the attrs. - g_preReadTermios = newTermios; + return true; } + } + + return TcSetAttr(&termios, blockIfBackground); +} + +void UninitializeTerminal() +{ + assert(g_haveInitTermios); - newTermios.c_iflag &= (uint32_t)(~(IXON | IXOFF)); - newTermios.c_lflag &= (uint32_t)(~(ECHO | ICANON | IEXTEN)); - newTermios.c_cc[VMIN] = minChars; - newTermios.c_cc[VTIME] = decisecondsTimeout; - IncorporateBreak(&newTermios, g_signalForBreak); + // This method is called on SIGQUIT/SIGINT from the signal dispatching thread + // and on atexit. - if (tcsetattr(STDIN_FILENO, TCSANOW, &newTermios) >= 0) + if (pthread_mutex_lock(&g_lock) == 0) + { + if (!g_terminalUninitialized) { - g_currTermios = newTermios; - g_readInProgress = true; + TcSetAttr(&g_initTermios, /* blockIfBackground */ false); + + g_terminalUninitialized = true; } + + pthread_mutex_unlock(&g_lock); + } +} + +void SystemNative_InitializeConsoleBeforeRead(uint8_t minChars, uint8_t decisecondsTimeout) +{ + if (pthread_mutex_lock(&g_lock) == 0) + { + g_reading = true; + + ConfigureTerminal(g_signalForBreak, /* forChild */ false, minChars, decisecondsTimeout, /* blockIfBackground */ true); + + pthread_mutex_unlock(&g_lock); } } void SystemNative_UninitializeConsoleAfterRead() { - if (g_readInProgress) + if (pthread_mutex_lock(&g_lock) == 0) + { + g_reading = false; + + pthread_mutex_unlock(&g_lock); + } +} + +void SystemNative_ConfigureTerminalForChildProcess(int32_t childUsesTerminal) +{ + assert(childUsesTerminal == 0 || childUsesTerminal == 1); + + if (pthread_mutex_lock(&g_lock) == 0) { - g_readInProgress = false; - - int tmpErrno = errno; // preserve any errors from before uninitializing - IncorporateBreak(&g_preReadTermios, g_signalForBreak); - int ret = tcsetattr(STDIN_FILENO, TCSANOW, &g_preReadTermios); - assert(ret >= 0); // shouldn't fail, but if it does we don't want to fail in release - (void)ret; - errno = tmpErrno; + // If the application is performing a read, assume the child process won't use the terminal. + if (g_reading) + { + return; + } + + // If no more children are using the terminal, invalidate our cached termios. + if (!childUsesTerminal) + { + g_hasCurrentTermios = false; + } + + 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) + { + WriteKeypadXmit(); + } + + pthread_mutex_unlock(&g_lock); } } @@ -289,57 +404,84 @@ int32_t SystemNative_SetSignalForBreak(int32_t signalForBreak) { assert(signalForBreak == 0 || signalForBreak == 1); - struct termios current; - if (tcgetattr(STDIN_FILENO, ¤t) >= 0) + int rv = 0; + + if (pthread_mutex_lock(&g_lock) == 0) { - IncorporateBreak(¤t, signalForBreak); - if (tcsetattr(STDIN_FILENO, TCSANOW, ¤t) >= 0) + if (ConfigureTerminal(signalForBreak, /* forChild */ false, /* minChars */ 1, /* decisecondsTimeout */ 0, /* blockIfBackground */ true)) { g_signalForBreak = signalForBreak; - return 1; + rv = 1; } + + pthread_mutex_unlock(&g_lock); } - return 0; + return rv; } -void ReinitializeConsole() +void ReinitializeTerminal() { - // pal_signal.cpp calls this on SIGCONT/SIGCHLD. - // This can happen when SystemNative_InitializeConsole was not called. - // This gets called on a signal handler, we may only use async-signal-safe functions. - - // If the process was suspended while reading, we need to - // re-initialize the console for the read, as the attributes - // previously set were likely overwritten. - if (g_readInProgress) + // Restores the state of the terminal after being suspended. + // pal_signal.cpp calls this on SIGCONT from the signal handling thread. + + if (pthread_mutex_lock(&g_lock) == 0) { - IncorporateBreak(&g_currTermios, g_signalForBreak); - tcsetattr(STDIN_FILENO, TCSANOW, &g_currTermios); - } + if (!g_childUsesTerminal) + { + if (g_hasCurrentTermios) + { + TcSetAttr(&g_currentTermios, /* blockIfBackground */ false); + } - // "Application mode" will also have been reset and needs to be redone. - WriteKeypadXmit(); + WriteKeypadXmit(); + } + + pthread_mutex_unlock(&g_lock); + } } -int32_t SystemNative_InitializeConsole() +static bool InitializeTerminalCore() { - if (!InitializeSignalHandling()) + g_haveInitTermios = tcgetattr(STDIN_FILENO, &g_initTermios) >= 0; + + if (!g_haveInitTermios && errno == ENOTTY) { - return 0; + g_noTty = true; } - if (tcgetattr(STDIN_FILENO, &g_initTermios) >= 0) + if (g_haveInitTermios) { - g_haveInitTermios = true; - g_signalForBreak = (g_initTermios.c_lflag & ISIG) != 0; + g_hasCurrentTermios = true; + g_currentTermios = g_initTermios; + g_signalForBreak = g_initTermios.c_lflag & (uint32_t)ISIG; + + atexit(UninitializeTerminal); } else { - g_haveInitTermios = false; g_signalForBreak = true; } - atexit(UninitializeConsole); - return 1; + return g_haveInitTermios || g_noTty; +} + +int32_t SystemNative_InitializeTerminalAndSignalHandling() +{ + static int32_t initialized = 0; + + // Both the Process and Console class call this method for initialization. + if (pthread_mutex_lock(&g_lock) == 0) + { + if (initialized == 0) + { + if (InitializeTerminalCore()) + { + initialized = InitializeSignalHandlingCore(); + } + } + pthread_mutex_unlock(&g_lock); + } + + return initialized; } diff --git a/src/libraries/Native/Unix/System.Native/pal_console.h b/src/libraries/Native/Unix/System.Native/pal_console.h index 83b172a..f5b59c0 100644 --- a/src/libraries/Native/Unix/System.Native/pal_console.h +++ b/src/libraries/Native/Unix/System.Native/pal_console.h @@ -58,11 +58,11 @@ DLLEXPORT int32_t SystemNative_GetWindowSize(WinSize* windowsSize); DLLEXPORT int32_t SystemNative_IsATty(intptr_t fd); /** - * Initializes the console for use by System.Console. + * Initializes signal handling and terminal for use by System.Console and System.Diagnostics.Process. * * Returns 1 on success; otherwise returns 0 and sets errno. */ -DLLEXPORT int32_t SystemNative_InitializeConsole(void); +DLLEXPORT int32_t SystemNative_InitializeTerminalAndSignalHandling(void); /** * Stores the string that can be written to stdout to transition @@ -90,16 +90,21 @@ DLLEXPORT void SystemNative_GetControlCharacters( DLLEXPORT int32_t SystemNative_StdinReady(void); /** - * Initializes the terminal in preparation for a read operation. + * Configures the terminal for System.Console Read. */ DLLEXPORT void SystemNative_InitializeConsoleBeforeRead(uint8_t minChars, uint8_t decisecondsTimeout); /** - * Restores the terminal's attributes to what they were before InitializeConsoleBeforeRead was called. + * Configures the terminal after System.Console Read. */ DLLEXPORT void SystemNative_UninitializeConsoleAfterRead(void); /** + * Configures the terminal for child processes. + */ +DLLEXPORT void SystemNative_ConfigureTerminalForChildProcess(int32_t enable); + +/** * Reads the number of bytes specified into the provided buffer from stdin. * Returns the number of bytes read on success; otherwise, -1 is returned an errno is set. */ @@ -129,9 +134,9 @@ typedef void (*CtrlCallback)(CtrlCode signalCode); /** * Called by pal_signal.cpp to reinitialize the console on SIGCONT/SIGCHLD. */ -void ReinitializeConsole(void); +void ReinitializeTerminal(void); /** * Called by pal_signal.cpp to uninitialize the console on SIGINT/SIGQUIT. */ -void UninitializeConsole(void); +void UninitializeTerminal(void); diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.c b/src/libraries/Native/Unix/System.Native/pal_signal.c index 7b18be6..2c16dba 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.c +++ b/src/libraries/Native/Unix/System.Native/pal_signal.c @@ -43,28 +43,15 @@ static struct sigaction* OrigActionFor(int sig) static void SignalHandler(int sig, siginfo_t* siginfo, void* context) { - if (sig == SIGCONT || sig == SIGCHLD) - { - // SIGCONT will be sent when we're resumed after suspension, at which point - // we need to set the terminal back up. Similarly, SIGCHLD will be sent after - // a child process completes, and that child could have left things in a bad state, - // so we similarly need to reinitialize. - ReinitializeConsole(); - } - // Signal handler for signals where we want our background thread to do the real processing. // It simply writes the signal code to a pipe that's read by the thread. - if (sig == SIGQUIT || sig == SIGINT || sig == SIGCHLD || sig == SIGWINCH) - { - // Write the signal code to the pipe - uint8_t signalCodeByte = (uint8_t)sig; - ssize_t writtenBytes; - while ((writtenBytes = write(g_signalPipe[1], &signalCodeByte, 1)) < 0 && errno == EINTR); + uint8_t signalCodeByte = (uint8_t)sig; + ssize_t writtenBytes; + while ((writtenBytes = write(g_signalPipe[1], &signalCodeByte, 1)) < 0 && errno == EINTR); - if (writtenBytes != 1) - { - abort(); // fatal error - } + if (writtenBytes != 1) + { + abort(); // fatal error } // Delegate to any saved handler we may have @@ -166,6 +153,10 @@ static void* SignalHandlerLoop(void* arg) callback(reapAll ? 1 : 0); } } + else if (signalCode == SIGCONT) + { + ReinitializeTerminal(); + } else if (signalCode != SIGWINCH) { assert_msg(false, "invalid signalCode", (int)signalCode); @@ -199,7 +190,7 @@ void SystemNative_UnregisterForCtrl() void SystemNative_RestoreAndHandleCtrl(CtrlCode ctrlCode) { int signalCode = ctrlCode == Break ? SIGQUIT : SIGINT; - UninitializeConsole(); + UninitializeTerminal(); sigaction(signalCode, OrigActionFor(signalCode), NULL); kill(getpid(), signalCode); } @@ -211,13 +202,8 @@ void SystemNative_SetTerminalInvalidationHandler(TerminalInvalidationCallback ca g_terminalInvalidationCallback = callback; } -uint32_t SystemNative_RegisterForSigChld(SigChldCallback callback) +void SystemNative_RegisterForSigChld(SigChldCallback callback) { - if (!InitializeSignalHandling()) - { - return 0; - } - assert(callback != NULL); assert(g_sigChldCallback == NULL); @@ -226,8 +212,6 @@ uint32_t SystemNative_RegisterForSigChld(SigChldCallback callback) g_sigChldCallback = callback; } pthread_mutex_unlock(&lock); - - return 1; } static void InstallSignalHandler(int sig, bool skipWhenSigIgn) @@ -255,7 +239,7 @@ static void InstallSignalHandler(int sig, bool skipWhenSigIgn) assert(rv == 0); } -static bool InitializeSignalHandlingCore() +int32_t InitializeSignalHandlingCore() { // Create a pipe we'll use to communicate with our worker // thread. We can't do anything interesting in the signal handler, @@ -263,7 +247,7 @@ static bool InitializeSignalHandlingCore() // the handling work. if (SystemNative_Pipe(g_signalPipe, PAL_O_CLOEXEC) != 0) { - return false; + return 0; } assert(g_signalPipe[0] >= 0); assert(g_signalPipe[1] >= 0); @@ -274,7 +258,7 @@ static bool InitializeSignalHandlingCore() { CloseSignalHandlingPipe(); errno = ENOMEM; - return false; + return 0; } *readFdPtr = g_signalPipe[0]; @@ -286,7 +270,7 @@ static bool InitializeSignalHandlingCore() free(readFdPtr); CloseSignalHandlingPipe(); errno = err; - return false; + return 0; } // Finally, register our signal handlers @@ -298,21 +282,5 @@ static bool InitializeSignalHandlingCore() InstallSignalHandler(SIGCHLD, /* skipWhenSigIgn */ false); InstallSignalHandler(SIGWINCH, /* skipWhenSigIgn */ false); - return true; -} - -uint32_t InitializeSignalHandling() -{ - static bool initialized = false; - - pthread_mutex_lock(&lock); - { - if (!initialized) - { - initialized = InitializeSignalHandlingCore(); - } - } - pthread_mutex_unlock(&lock); - - return initialized ? 1 : 0; + return 1; } diff --git a/src/libraries/Native/Unix/System.Native/pal_signal.h b/src/libraries/Native/Unix/System.Native/pal_signal.h index 09eca3c..41e51e0 100644 --- a/src/libraries/Native/Unix/System.Native/pal_signal.h +++ b/src/libraries/Native/Unix/System.Native/pal_signal.h @@ -8,11 +8,11 @@ #include "pal_types.h" /** - * Initializes the signal handling for use by System.Console and System.Process. + * Initializes the signal handling, called by InitializeTerminalAndSignalHandling. * * Returns 1 on success; otherwise returns 0 and sets errno. */ -uint32_t InitializeSignalHandling(void); +int32_t InitializeSignalHandlingCore(void); /** * Hooks up the specified callback for notifications when SIGINT or SIGQUIT is received. @@ -41,12 +41,9 @@ typedef void (*SigChldCallback)(int reapAll); /** * Hooks up the specified callback for notifications when SIGCHLD is received. * - * Not thread safe. Caller must provide its owns synchronization to ensure RegisterForSigChld - * is not called concurrently with itself. - * * Should only be called when a callback is not currently registered. */ -DLLEXPORT uint32_t SystemNative_RegisterForSigChld(SigChldCallback callback); +DLLEXPORT void SystemNative_RegisterForSigChld(SigChldCallback callback); /** * Remove our handler and reissue the signal to be picked up by the previously registered handler. diff --git a/src/libraries/System.Console/src/System.Console.csproj b/src/libraries/System.Console/src/System.Console.csproj index d4cd654..13f7536 100644 --- a/src/libraries/System.Console/src/System.Console.csproj +++ b/src/libraries/System.Console/src/System.Console.csproj @@ -259,8 +259,8 @@ Common\Interop\Unix\Interop.GetWindowWidth.cs - - Common\Interop\Unix\Interop.InitializeConsole.cs + + Common\Interop\Unix\Interop.InitializeTerminalAndSignalHandling.cs Common\Interop\Unix\Interop.ReadStdinUnbuffered.cs diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index 7aa9595..082089d 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs @@ -922,9 +922,7 @@ namespace System { if (!s_initialized) { - // Ensure the console is configured appropriately. This will start - // signal handlers, etc. - if (!Interop.Sys.InitializeConsole()) + if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { throw new Win32Exception(); } diff --git a/src/libraries/System.Console/src/System/IO/StdInReader.cs b/src/libraries/System.Console/src/System/IO/StdInReader.cs index eda2d9a..bc8bee2 100644 --- a/src/libraries/System.Console/src/System/IO/StdInReader.cs +++ b/src/libraries/System.Console/src/System/IO/StdInReader.cs @@ -87,8 +87,7 @@ namespace System.IO Debug.Assert(_tmpKeys.Count == 0); string readLineStr = null; - // Disable echo and buffering. These will be disabled for the duration of the line read. - Interop.Sys.InitializeConsoleBeforeRead(); + Interop.Sys.InitializeConsoleBeforeRead(); try { // Read key-by-key until we've read a line. diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 73a2179..814c388 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -311,6 +311,9 @@ Common\Interop\Unix\Interop.SysConf.cs + + Common\Interop\Unix\Interop.ConfigureTerminalForChildProcess.cs + Common\Interop\Unix\Interop.ForkAndExecProcess.cs @@ -335,6 +338,9 @@ Common\Interop\Unix\Interop.GetUid.cs + + Common\Interop\Unix\Interop.InitializeTerminalAndSignalHandling.cs + Common\Interop\Unix\Interop.Kill.cs diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 804adb7..de7c53e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -19,10 +19,11 @@ namespace System.Diagnostics { private static readonly UTF8Encoding s_utf8NoBom = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false); - private static volatile bool s_sigchildHandlerRegistered = false; - private static readonly object s_sigchildGate = new object(); + private static volatile bool s_initialized = false; + private static readonly object s_initializedGate = new object(); private static readonly Interop.Sys.SigChldCallback s_sigChildHandler = OnSigChild; private static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); + private static int s_childrenUsingTerminalCount; /// /// Puts a Process component in state to interact with operating system processes that run in a @@ -367,7 +368,7 @@ namespace System.Diagnostics /// The start info with which to start the process. private bool StartCore(ProcessStartInfo startInfo) { - EnsureSigChildHandler(); + EnsureInitialized(); string filename; string[] argv; @@ -393,6 +394,13 @@ namespace System.Diagnostics (userId, groupId, groups) = GetUserAndGroupIds(startInfo); } + // .NET applications don't echo characters unless there is a Console.Read operation. + // Unix applications expect the terminal to be in an echoing state by default. + // To support processes that interact with the terminal (e.g. 'vi'), we need to configure the + // terminal to echo. We keep this configuration as long as there are children possibly using the terminal. + // We consider the child to be interactively using the terminal when both stdin and stdout are connected. + bool usesTerminal = !startInfo.RedirectStandardInput && !startInfo.RedirectStandardOutput; + if (startInfo.UseShellExecute) { string verb = startInfo.Verb; @@ -416,7 +424,7 @@ namespace System.Diagnostics isExecuting = ForkAndExecProcess(filename, argv, envp, cwd, startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, - out stdinFd, out stdoutFd, out stderrFd, + out stdinFd, out stdoutFd, out stderrFd, usesTerminal, throwOnNoExec: false); // return false instead of throwing on ENOEXEC } @@ -429,7 +437,7 @@ namespace System.Diagnostics ForkAndExecProcess(filename, argv, envp, cwd, startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, - out stdinFd, out stdoutFd, out stderrFd); + out stdinFd, out stdoutFd, out stderrFd, usesTerminal); } } else @@ -444,7 +452,7 @@ namespace System.Diagnostics ForkAndExecProcess(filename, argv, envp, cwd, startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, - out stdinFd, out stdoutFd, out stderrFd); + out stdinFd, out stdoutFd, out stderrFd, usesTerminal); } // Configure the parent's ends of the redirection streams. @@ -479,7 +487,7 @@ namespace System.Diagnostics bool redirectStdin, bool redirectStdout, bool redirectStderr, bool setCredentials, uint userId, uint groupId, uint[] groups, out int stdinFd, out int stdoutFd, out int stderrFd, - bool throwOnNoExec = true) + bool usesTerminal, bool throwOnNoExec = true) { if (string.IsNullOrEmpty(filename)) { @@ -491,6 +499,11 @@ namespace System.Diagnostics s_processStartLock.EnterReadLock(); try { + if (usesTerminal) + { + ConfigureTerminalForChildProcesses(1); + } + int childPid; // Invoke the shim fork/execve routine. It will create pipes for all requested @@ -509,7 +522,7 @@ namespace System.Diagnostics { // Ensure we'll reap this process. // note: SetProcessId will set this if we don't set it first. - _waitStateHolder = new ProcessWaitState.Holder(childPid, isNewChild: true); + _waitStateHolder = new ProcessWaitState.Holder(childPid, isNewChild: true, usesTerminal); // Store the child's information into this Process object. Debug.Assert(childPid >= 0); @@ -532,6 +545,14 @@ namespace System.Diagnostics finally { s_processStartLock.ExitReadLock(); + + if (_waitStateHolder == null && usesTerminal) + { + // We failed to launch a child that could use the terminal. + s_processStartLock.EnterWriteLock(); + ConfigureTerminalForChildProcesses(-1); + s_processStartLock.ExitWriteLock(); + } } } @@ -973,24 +994,26 @@ namespace System.Diagnostics private bool WaitForInputIdleCore(int milliseconds) => throw new InvalidOperationException(SR.InputIdleUnkownError); - private static void EnsureSigChildHandler() + private static void EnsureInitialized() { - if (s_sigchildHandlerRegistered) + if (s_initialized) { return; } - lock (s_sigchildGate) + lock (s_initializedGate) { - if (!s_sigchildHandlerRegistered) + if (!s_initialized) { - // Ensure signal handling is setup and register our callback. - if (!Interop.Sys.RegisterForSigChld(s_sigChildHandler)) + if (!Interop.Sys.InitializeTerminalAndSignalHandling()) { throw new Win32Exception(); } - s_sigchildHandlerRegistered = true; + // Register our callback. + Interop.Sys.RegisterForSigChld(s_sigChildHandler); + + s_initialized = true; } } } @@ -1008,5 +1031,33 @@ namespace System.Diagnostics s_processStartLock.ExitWriteLock(); } } + + /// + /// This method is called when the number of child processes that are using the terminal changes. + /// It updates the terminal configuration if necessary. + /// + internal static void ConfigureTerminalForChildProcesses(int increment) + { + Debug.Assert(increment != 0); + + int childrenUsingTerminalRemaining = Interlocked.Add(ref s_childrenUsingTerminalCount, increment); + if (increment > 0) + { + Debug.Assert(s_processStartLock.IsReadLockHeld); + + // At least one child is using the terminal. + Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: true); + } + else + { + Debug.Assert(s_processStartLock.IsWriteLockHeld); + + if (childrenUsingTerminalRemaining == 0) + { + // No more children are using the terminal. + Interop.Sys.ConfigureTerminalForChildProcess(childUsesTerminal: false); + } + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 2aeec4b..4b90e19 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -55,9 +55,9 @@ namespace System.Diagnostics { internal ProcessWaitState _state; - internal Holder(int processId, bool isNewChild = false) + internal Holder(int processId, bool isNewChild = false, bool usesTerminal = false) { - _state = ProcessWaitState.AddRef(processId, isNewChild); + _state = ProcessWaitState.AddRef(processId, isNewChild, usesTerminal); } ~Holder() @@ -99,7 +99,7 @@ namespace System.Diagnostics /// /// The process ID for which we need wait state. /// The wait state object. - internal static ProcessWaitState AddRef(int processId, bool isNewChild) + internal static ProcessWaitState AddRef(int processId, bool isNewChild, bool usesTerminal) { lock (s_childProcessWaitStates) { @@ -109,7 +109,7 @@ namespace System.Diagnostics // When the PID is recycled for a new child, we remove the old child. s_childProcessWaitStates.Remove(processId); - pws = new ProcessWaitState(processId, isChild: true); + pws = new ProcessWaitState(processId, isChild: true, usesTerminal); s_childProcessWaitStates.Add(processId, pws); pws._outstandingRefCount++; // For Holder pws._outstandingRefCount++; // Decremented in CheckChildren @@ -143,7 +143,7 @@ namespace System.Diagnostics } if (pws == null) { - pws = new ProcessWaitState(processId, isChild: false, exitTime); + pws = new ProcessWaitState(processId, isChild: false, usesTerminal: false, exitTime); s_processWaitStates.Add(processId, pws); } pws._outstandingRefCount++; @@ -196,6 +196,8 @@ namespace System.Diagnostics private readonly int _processId; /// Associated process is a child process. private readonly bool _isChild; + /// Associated process is a child that can use the terminal. + private readonly bool _usesTerminal; /// If a wait operation is in progress, the Task that represents it; otherwise, null. private Task _waitInProgress; @@ -216,11 +218,12 @@ namespace System.Diagnostics /// Initialize the wait state object. /// The associated process' ID. - private ProcessWaitState(int processId, bool isChild, DateTime exitTime = default) + private ProcessWaitState(int processId, bool isChild, bool usesTerminal, DateTime exitTime = default) { Debug.Assert(processId >= 0); _processId = processId; _isChild = isChild; + _usesTerminal = usesTerminal; _exitTime = exitTime; } @@ -562,7 +565,14 @@ namespace System.Diagnostics { _exitCode = exitCode; + if (_usesTerminal) + { + // Update terminal settings before calling SetExited. + Process.ConfigureTerminalForChildProcesses(-1); + } + SetExited(); + return true; } else if (waitResult == 0) -- 2.7.4