From 121e945f3d171408155eb00da4bf918dc69e2e79 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 20 Mar 2015 09:56:28 +0000 Subject: [PATCH 1/2] Emulate GenerateConsoleCtrlEvent() upon Ctrl+C When a process is terminated via TerminateProcess(), it has no chance to do anything in the way of cleaning up. This is particularly noticeable when a lengthy Git for Windows process tries to update Git's index file and leaves behind an index.lock file. Git's idea is to remove the stale index.lock file in that case, using the signal and atexit handlers available in Linux. But those signal handlers never run. Note: this is not an issue for MSYS2 processes because MSYS2 emulates Unix' signal system accurately, both for the process sending the kill signal and the process receiving it. Win32 processes do not have such a signal handler, though, instead MSYS2 shuts them down via `TerminateProcess()`. For a while, Git for Windows tried to use a gentler method, described in the Dr Dobb's article "A Safer Alternative to TerminateProcess()" by Andrew Tucker (July 1, 1999), http://www.drdobbs.com/a-safer-alternative-to-terminateprocess/184416547 Essentially, we injected a new thread into the running process that does nothing else than running the ExitProcess() function. However, this was still not in line with the way CMD handles Ctrl+C: it gives processes a chance to do something upon Ctrl+C by calling SetConsoleCtrlHandler(), and ExitProcess() simply never calls that handler. So for a while we tried to handle SIGINT/SIGTERM by attaching to the console of the command to interrupt, and generating the very same event as CMD does via GenerateConsoleCtrlEvent(). This method *still* was not correct, though, as it would interrupt *every* process attached to that Console, not just the process (and its children) that we wanted to signal. A symptom was that hitting Ctrl+C while `git log` was shown in the pager would interrupt *the pager*. The method we settled on is to emulate what GenerateConsoleCtrlEvent() does, but on a process by process basis: inject a remote thread and call the (private) function kernel32!CtrlRoutine. To obtain said function's address, we use the dbghelp API to generate a stack trace from a handler configured via SetConsoleCtrlHandler() and triggered via GenerateConsoleCtrlEvent(). To avoid killing each and all processes attached to the same Console as the MSYS2 runtime, we modify the cygwin-console-helper to optionally print the address of kernel32!CtrlRoutine to stdout, and then spawn it with a new Console. Note that this also opens the door to handling 32-bit process from a 64-bit MSYS2 runtime and vice versa, by letting the MSYS2 runtime look for the cygwin-console-helper.exe of the "other architecture" in a specific place (we choose /usr/libexec/, as it seems to be the convention for helper .exe files that are not intended for public consumption). The 32-bit helper implicitly links to libgcc_s_dw2.dll and libwinpthread-1.dll, so to avoid cluttering /usr/libexec/, we look for the helped of the "other" architecture in the corresponding mingw32/ or mingw64/ subdirectory. Among other bugs, this strategy to handle Ctrl+C fixes the MSYS2 side of the bug where interrupting `git clone https://...` would send the spawned-off `git remote-https` process into the background instead of interrupting it, i.e. the clone would continue and its progress would be reported mercilessly to the console window without the user being able to do anything about it (short of firing up the task manager and killing the appropriate task manually). Note that this special-handling is only necessary when *MSYS2* handles the Ctrl+C event, e.g. when interrupting a process started from within MinTTY or any other non-cmd-based terminal emulator. If the process was started from within `cmd.exe`'s terminal window, child processes are already killed appropriately upon Ctrl+C, by `cmd.exe` itself. Signed-off-by: Johannes Schindelin --- winsup/cygwin/exceptions.cc | 20 +- winsup/cygwin/include/cygwin/exit_process.h | 371 ++++++++++++++++++++ winsup/cygwin/include/cygwin/signal.h | 1 + 3 files changed, 390 insertions(+), 2 deletions(-) create mode 100644 winsup/cygwin/include/cygwin/exit_process.h diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc index d3ac51673a..267c216e2d 100644 --- a/winsup/cygwin/exceptions.cc +++ b/winsup/cygwin/exceptions.cc @@ -28,6 +28,7 @@ details. */ #include "ntdll.h" #include "exception.h" #include "posix_timer.h" +#include "cygwin/exit_process.h" /* Definitions for code simplification */ #ifdef __x86_64__ @@ -1545,8 +1546,23 @@ sigpacket::process () dosig: if (have_execed) { - sigproc_printf ("terminating captive process"); - TerminateProcess (ch_spawn, sigExeced = si.si_signo); + switch (si.si_signo) + { + case SIGUSR1: + case SIGUSR2: + case SIGCONT: + case SIGSTOP: + case SIGTSTP: + case SIGTTIN: + case SIGTTOU: + system_printf ("Suppressing signal %d to win32 process (pid %u)", + (int)si.si_signo, (unsigned int)GetProcessId(ch_spawn)); + goto done; + default: + sigproc_printf ("terminating captive process"); + rc = exit_process_tree (ch_spawn, 128 + (sigExeced = si.si_signo)); + goto done; + } } /* Dispatch to the appropriate function. */ sigproc_printf ("signal %d, signal handler %p", si.si_signo, handler); diff --git a/winsup/cygwin/include/cygwin/exit_process.h b/winsup/cygwin/include/cygwin/exit_process.h new file mode 100644 index 0000000000..e0981bbe4a --- /dev/null +++ b/winsup/cygwin/include/cygwin/exit_process.h @@ -0,0 +1,371 @@ +#ifndef EXIT_PROCESS_H +#define EXIT_PROCESS_H + +/* + * This file contains functions to terminate a Win32 process, as gently as + * possible. + * + * If appropriate, we will attempt to emulate a console Ctrl event for the + * process and its (transitive) children. Otherwise we will fall back to + * terminating the process(es). + * + * As we do not want to export this function in the MSYS2 runtime, these + * functions are marked as file-local. + * + * The idea is to inject a thread into the given process that runs either + * kernel32!CtrlRoutine() (i.e. the work horse of GenerateConsoleCtrlEvent()) + * for SIGINT (Ctrl+C) and SIGQUIT (Ctrl+Break), or ExitProcess() for SIGTERM. + * + * For SIGKILL, we run TerminateProcess() without injecting anything, and this + * is also the fall-back when the previous methods are unavailable. + * + * Note: as kernel32.dll is loaded before any process, the other process and + * this process will have ExitProcess() at the same address. The same holds + * true for kernel32!CtrlRoutine(), of course, but it is an internal API + * function, so we cannot look it up directly. Instead, we launch + * cygwin-console-helper.exe to find out (which has been modified to offer the + * option to print the address to stdout). + * + * This function expects the process handle to have the access rights for + * CreateRemoteThread(): PROCESS_CREATE_THREAD, PROCESS_QUERY_INFORMATION, + * PROCESS_VM_OPERATION, PROCESS_VM_WRITE, and PROCESS_VM_READ. + * + * The idea for the injected remote thread comes from the Dr Dobb's article "A + * Safer Alternative to TerminateProcess()" by Andrew Tucker (July 1, 1999), + * http://www.drdobbs.com/a-safer-alternative-to-terminateprocess/184416547. + * + * The idea to use kernel32!CtrlRoutine for the other signals comes from + * SendSignal (https://github.com/AutoSQA/SendSignal/ and + * http://stanislavs.org/stopping-command-line-applications-programatically-with-ctrl-c-events-from-net/). + */ + +#include + +#ifndef __INSIDE_CYGWIN__ +/* To help debugging via kill.exe */ +#define small_printf(...) fprintf(stderr, __VA_ARGS__) +#endif + +static LPTHREAD_START_ROUTINE +get_address_from_cygwin_console_helper(int bitness, wchar_t *function_name) +{ + const char *name; + if (bitness == 32) + name = "/usr/libexec/getprocaddr32.exe"; + else if (bitness == 64) + name = "/usr/libexec/getprocaddr64.exe"; + else + return NULL; /* what?!? */ + wchar_t wbuf[PATH_MAX]; + if (cygwin_conv_path (CCP_POSIX_TO_WIN_W, name, wbuf, PATH_MAX) || + GetFileAttributesW (wbuf) == INVALID_FILE_ATTRIBUTES) + return NULL; + + HANDLE h[2]; + SECURITY_ATTRIBUTES attrs; + attrs.nLength = sizeof(attrs); + attrs.bInheritHandle = TRUE; + attrs.lpSecurityDescriptor = NULL; + if (!CreatePipe(&h[0], &h[1], &attrs, 0)) + return NULL; + + STARTUPINFOW si = {}; + PROCESS_INFORMATION pi; + size_t len = wcslen (wbuf) + wcslen (function_name) + 1; + WCHAR cmd[len + 1]; + WCHAR title[] = L"cygwin-console-helper"; + + swprintf (cmd, len + 1, L"%S %S", wbuf, function_name); + + si.cb = sizeof (si); + si.dwFlags = STARTF_USESHOWWINDOW | STARTF_USESTDHANDLES; + si.wShowWindow = SW_HIDE; + si.lpTitle = title; + si.hStdInput = si.hStdError = INVALID_HANDLE_VALUE; + si.hStdOutput = h[1]; + + /* Create a new hidden process. Use the two event handles as + argv[1] and argv[2]. */ + BOOL x = CreateProcessW (NULL, cmd, NULL, NULL, TRUE, + CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP, + NULL, NULL, &si, &pi); + CloseHandle(h[1]); + if (!x) + { + CloseHandle(h[0]); + return NULL; + } + + CloseHandle (pi.hThread); + CloseHandle (pi.hProcess); + + char buffer[64]; + DWORD offset = 0, count = 0; + while (offset < sizeof(buffer) - 1) + { + if (!ReadFile(h[0], buffer + offset, 1, &count, NULL)) + { + offset = 0; + break; + } + if (!count || buffer[offset] == '\n') + break; + offset += count; + } + CloseHandle(h[0]); + + buffer[offset] = '\0'; + + return (LPTHREAD_START_ROUTINE) strtoull(buffer, NULL, 16); +} + +static int current_is_wow = -1; +static int is_32_bit_os = -1; + +static BOOL get_wow(HANDLE process, BOOL& is_wow, int& bitness) +{ + if (is_32_bit_os == -1) + { + SYSTEM_INFO info; + + GetNativeSystemInfo(&info); + if (info.wProcessorArchitecture == 0) + is_32_bit_os = 1; + else if (info.wProcessorArchitecture == 9) + is_32_bit_os = 0; + else + is_32_bit_os = -2; + } + + if (current_is_wow == -1 && + !IsWow64Process (GetCurrentProcess (), ¤t_is_wow)) + current_is_wow = -2; + + if (is_32_bit_os == -2 || current_is_wow == -2) + return FALSE; + + if (!IsWow64Process (process, &is_wow)) + return FALSE; + + bitness = is_32_bit_os || is_wow ? 32 : 64; + return TRUE; +} + +static LPTHREAD_START_ROUTINE +get_exit_process_address_for_process(HANDLE process) +{ + BOOL is_wow; + int bitness; + if (!get_wow (process, is_wow, bitness)) + return NULL; /* could not determine */ + + if (is_wow == current_is_wow) + { + static LPTHREAD_START_ROUTINE exit_process_address; + if (!exit_process_address) + { + HINSTANCE kernel32 = GetModuleHandle ("kernel32"); + exit_process_address = (LPTHREAD_START_ROUTINE) + GetProcAddress (kernel32, "ExitProcess"); + } + return exit_process_address; + } + + /* Trying to terminate a 32-bit process from 64-bit or vice versa */ + static LPTHREAD_START_ROUTINE exit_process_address; + if (!exit_process_address) + { + exit_process_address = + get_address_from_cygwin_console_helper(bitness, L"ExitProcess"); + } + return exit_process_address; +} + +static LPTHREAD_START_ROUTINE +get_ctrl_routine_address_for_process(HANDLE process) +{ + BOOL is_wow; + int bitness; + if (!get_wow (process, is_wow, bitness)) + return NULL; /* could not determine */ + + if (bitness == 64) + { + static LPTHREAD_START_ROUTINE ctrl_routine_address; + if (!ctrl_routine_address) + ctrl_routine_address = + get_address_from_cygwin_console_helper(64, L"CtrlRoutine"); + return ctrl_routine_address; + } + + static LPTHREAD_START_ROUTINE ctrl_routine_address; + if (!ctrl_routine_address) + { + ctrl_routine_address = + get_address_from_cygwin_console_helper(32, L"CtrlRoutine"); + } + return ctrl_routine_address; +} + +static int +inject_remote_thread_into_process(HANDLE process, LPTHREAD_START_ROUTINE address, int exit_code) +{ + int res = -1; + + if (!address) + return res; + + DWORD thread_id; + HANDLE thread = CreateRemoteThread (process, NULL, 1024 * 1024, address, + (PVOID) exit_code, 0, &thread_id); + if (thread) + { + /* + * Wait up to 10 seconds (arbitrary constant) for the thread to finish; + * After that grace period, fall back to terminating non-gently. + */ + if (WaitForSingleObject (thread, 10000) == WAIT_OBJECT_0) + res = 0; + CloseHandle (thread); + } + + return res; +} + +/** + * Terminates the process corresponding to the process ID + * + * This way of terminating the processes is not gentle: the process gets + * no chance of cleaning up after itself (closing file handles, removing + * .lock files, terminating spawned processes (if any), etc). + */ +static int +exit_one_process(HANDLE process, int exit_code) +{ + LPTHREAD_START_ROUTINE address = NULL; + int signo = exit_code & 0x7f; + + switch (signo) + { + case SIGINT: + case SIGQUIT: + address = get_ctrl_routine_address_for_process(process); + if (address && + !inject_remote_thread_into_process(process, address, + signo == SIGINT ? + CTRL_C_EVENT : CTRL_BREAK_EVENT)) + return 0; + /* fall-through */ + case SIGTERM: + address = get_exit_process_address_for_process(process); + if (address && !inject_remote_thread_into_process(process, address, exit_code)) + return 0; + break; + default: + break; + } + + return int(TerminateProcess (process, exit_code)); +} + +#include +#include + +/** + * Terminates the process corresponding to the process ID and all of its + * directly and indirectly spawned subprocesses using the + * exit_one_process() function. + */ +static int +exit_process_tree(HANDLE main_process, int exit_code) +{ + HANDLE snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPPROCESS, 0); + PROCESSENTRY32 entry; + DWORD pids[16384]; + int max_len = sizeof (pids) / sizeof (*pids), i, len, ret = 0; + pid_t pid = GetProcessId (main_process); + int signo = exit_code & 0x7f; + + pids[0] = (DWORD) pid; + len = 1; + + /* + * Even if Process32First()/Process32Next() seem to traverse the + * processes in topological order (i.e. parent processes before + * child processes), there is nothing in the Win32 API documentation + * suggesting that this is guaranteed. + * + * Therefore, run through them at least twice and stop when no more + * process IDs were added to the list. + */ + for (;;) + { + memset (&entry, 0, sizeof (entry)); + entry.dwSize = sizeof (entry); + + if (!Process32First (snapshot, &entry)) + break; + + int orig_len = len; + do + { + /** + * Look for the parent process ID in the list of pids to kill, and if + * found, add it to the list. + */ + for (i = len - 1; i >= 0; i--) + { + if (pids[i] == entry.th32ProcessID) + break; + if (pids[i] != entry.th32ParentProcessID) + continue; + + /* We found a process to kill; is it an MSYS2 process? */ + pid_t cyg_pid = cygwin_winpid_to_pid(entry.th32ProcessID); + if (cyg_pid > -1) + { + if (cyg_pid == getpgid (cyg_pid)) + kill(cyg_pid, signo); + break; + } + pids[len++] = entry.th32ProcessID; + break; + } + } + while (len < max_len && Process32Next (snapshot, &entry)); + + if (orig_len == len || len >= max_len) + break; + } + + for (i = len - 1; i >= 0; i--) + { + HANDLE process; + + if (!i) + process = main_process; + else + { + process = OpenProcess (PROCESS_CREATE_THREAD | + PROCESS_QUERY_INFORMATION | + PROCESS_VM_OPERATION | PROCESS_VM_WRITE | + PROCESS_VM_READ, FALSE, pids[i]); + if (!process) + process = OpenProcess (PROCESS_TERMINATE, FALSE, pids[i]); + } + DWORD code; + + if (process && + (!GetExitCodeProcess (process, &code) || code == STILL_ACTIVE)) + { + if (!exit_one_process (process, exit_code)) + ret = -1; + } + if (process) + CloseHandle (process); + } + + return ret; +} + +#endif diff --git a/winsup/cygwin/include/cygwin/signal.h b/winsup/cygwin/include/cygwin/signal.h index e659d7ae06..081b0a5d67 100644 --- a/winsup/cygwin/include/cygwin/signal.h +++ b/winsup/cygwin/include/cygwin/signal.h @@ -478,6 +478,7 @@ extern const char *sys_siglist[]; extern const char __declspec(dllimport) *sys_sigabbrev[]; extern const char __declspec(dllimport) *sys_siglist[]; #endif +void kill_process_tree(pid_t pid, int sig); #ifdef __cplusplus } From 20c57b5ce3f96a2bef6ae03ecfc40c384d15cd64 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 20 Mar 2015 10:01:50 +0000 Subject: [PATCH 2/2] kill: kill Win32 processes more gently This change is the equivalent to the change to the Ctrl+C handling we just made. Signed-off-by: Johannes Schindelin --- winsup/utils/kill.cc | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/winsup/utils/kill.cc b/winsup/utils/kill.cc index a22d70253f..45690516e5 100644 --- a/winsup/utils/kill.cc +++ b/winsup/utils/kill.cc @@ -17,6 +17,7 @@ details. */ #include #include #include +#include static char *prog_name; @@ -186,10 +187,20 @@ forcekill (pid_t pid, DWORD winpid, int sig, int wait) return; } if (!wait || WaitForSingleObject (h, 200) != WAIT_OBJECT_0) - if (sig && !TerminateProcess (h, sig << 8) - && WaitForSingleObject (h, 200) != WAIT_OBJECT_0) - fprintf (stderr, "%s: couldn't kill pid %u, %u\n", - prog_name, (unsigned int) dwpid, (unsigned int) GetLastError ()); + { + HANDLE cur = GetCurrentProcess (), h2; + /* duplicate handle with access rights required for exit_process_tree() */ + if (DuplicateHandle (cur, h, cur, &h2, PROCESS_CREATE_THREAD | + PROCESS_QUERY_INFORMATION | + PROCESS_VM_OPERATION | + PROCESS_VM_WRITE | PROCESS_VM_READ | + PROCESS_TERMINATE, FALSE, 0)) + { + CloseHandle(h); + h = h2; + } + exit_process_tree (h2, 128 + sig); + } CloseHandle (h); }