From 31d26854a1af278de774aca20926c41a031fe8d2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 16 Apr 2018 14:59:39 +0200 Subject: [PATCH 1/3] Add a helper to obtain a function's address in kernel32.dll In particular, we are interested in the address of the CtrlRoutine and the ExitProcess functions. Since kernel32.dll is loaded first thing, the addresses will be the same for all processes (matching the CPU architecture, of course). This will help us with emulating SIGINT properly (by not sending signals to *all* processes attached to the same Console). Signed-off-by: Johannes Schindelin --- winsup/utils/Makefile.in | 22 +++++- winsup/utils/configure | 89 ++++++++++++++++++++++ winsup/utils/configure.ac | 5 ++ winsup/utils/getprocaddr.c | 146 +++++++++++++++++++++++++++++++++++++ 4 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 winsup/utils/getprocaddr.c diff --git a/winsup/utils/Makefile.in b/winsup/utils/Makefile.in index b9cc985114..73b16eddc2 100644 --- a/winsup/utils/Makefile.in +++ b/winsup/utils/Makefile.in @@ -35,6 +35,7 @@ prefix:=@prefix@ exec_prefix:=@exec_prefix@ bindir:=@bindir@ +libexecdir = @libexecdir@ program_transform_name:=@program_transform_name@ override INSTALL:=@INSTALL@ @@ -51,6 +52,8 @@ CYGWIN_LDFLAGS := -static -Wl,--enable-auto-import -L${WINDOWS_LIBDIR} $(LDLIBS) DEP_LDLIBS := $(cygwin_build)/libmsys-2.0.a MINGW_CXX := @MINGW_CXX@ +MINGW32_CC := @MINGW32_CC@ +MINGW64_CC := @MINGW64_CC@ # List all binaries to be linked in Cygwin mode. Each binary on this list # must have a corresponding .o of the same name. @@ -121,7 +124,7 @@ else all: warn_dumper endif -all: Makefile $(CYGWIN_BINS) $(MINGW_BINS) +all: Makefile $(CYGWIN_BINS) $(MINGW_BINS) getprocaddr32.exe getprocaddr64.exe # test harness support (note: the "MINGW_BINS +=" should come after the # "all:" above so that the testsuite is not run for "make" but only @@ -160,6 +163,19 @@ $(CYGWIN_BINS): %.exe: %.o $(MINGW_BINS): $(DEP_LDLIBS) $(CYGWIN_BINS): $(DEP_LDLIBS) +# Must *not* use -O2 here, as it screws up the stack backtrace +getprocaddr32.o: %32.o: %.c + $(MINGW32_CC) -c -o $@ $< + +getprocaddr32.exe: %.exe: %.o + $(MINGW32_CC) -o $@ $^ -static -ldbghelp + +getprocaddr64.o: %64.o: %.c + $(MINGW64_CC) -c -o $@ $< + +getprocaddr64.exe: %.exe: %.o + $(MINGW64_CC) -o $@ $^ -static -ldbghelp + cygcheck.o cygpath.o module_info.o path.o ps.o regtool.o strace.o: loadlib.h .PHONY: clean @@ -177,6 +193,10 @@ install: all n=`echo $$i | sed '$(program_transform_name)'`; \ $(INSTALL_PROGRAM) $$i $(DESTDIR)$(bindir)/$$n; \ done + /bin/mkdir -p ${DESTDIR}${libexecdir} + for n in getprocaddr32 getprocaddr64; do \ + $(INSTALL_PROGRAM) $$n $(DESTDIR)$(libexecdir)/$$n; \ + done $(cygwin_build)/libmsys-2.0.a: $(cygwin_build)/Makefile @$(MAKE) -C $(@D) $(@F) diff --git a/winsup/utils/configure b/winsup/utils/configure index 32f75d6e00..7ee4ee47e7 100755 --- a/winsup/utils/configure +++ b/winsup/utils/configure @@ -589,6 +589,8 @@ ac_no_link=no ac_subst_vars='LTLIBOBJS LIBOBJS configure_args +MINGW64_CC +MINGW32_CC MINGW_CXX INSTALL_DATA INSTALL_SCRIPT @@ -3303,6 +3305,93 @@ done test -n "$MINGW_CXX" || as_fn_error $? "no acceptable mingw g++ found in \$PATH" "$LINENO" 5 +for ac_prog in i686-w64-mingw32-gcc +do + # Extract the first word of "$ac_prog", so it can be a program name with args. +set dummy $ac_prog; ac_word=$2 +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +$as_echo_n "checking for $ac_word... " >&6; } +if ${ac_cv_prog_MINGW32_CC+:} false; then : + $as_echo_n "(cached) " >&6 +else + if test -n "$MINGW32_CC"; then + ac_cv_prog_MINGW32_CC="$MINGW32_CC" # Let the user override the test. +else +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do + IFS=$as_save_IFS + test -z "$as_dir" && as_dir=. + for ac_exec_ext in '' $ac_executable_extensions; do + if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then + ac_cv_prog_MINGW32_CC="$ac_prog" + $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5 + break 2 + fi +done + done +IFS=$as_save_IFS + +fi +fi +MINGW32_CC=$ac_cv_prog_MINGW32_CC +if test -n "$MINGW32_CC"; then + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $MINGW32_CC" >&5 +$as_echo "$MINGW32_CC" >&6; } +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } +fi + + + test -n "$MINGW32_CC" && break +done + +test -n "$MINGW32_CC" || as_fn_error $? "no acceptable mingw32 gcc found in \$PATH" "$LINENO" 5 +for ac_prog in x86_64-w64-mingw32-gcc +do + # Extract the first word of "$ac_prog", so it can be a program name with args. +set dummy $ac_prog; ac_word=$2 +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +$as_echo_n "checking for $ac_word... " >&6; } +if ${ac_cv_prog_MINGW64_CC+:} false; then : + $as_echo_n "(cached) " >&6 +else + if test -n "$MINGW64_CC"; then + ac_cv_prog_MINGW64_CC="$MINGW64_CC" # Let the user override the test. +else +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do + IFS=$as_save_IFS + test -z "$as_dir" && as_dir=. + for ac_exec_ext in '' $ac_executable_extensions; do + if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then + ac_cv_prog_MINGW64_CC="$ac_prog" + $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5 + break 2 + fi +done + done +IFS=$as_save_IFS + +fi +fi +MINGW64_CC=$ac_cv_prog_MINGW64_CC +if test -n "$MINGW64_CC"; then + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $MINGW64_CC" >&5 +$as_echo "$MINGW64_CC" >&6; } +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } +fi + + + test -n "$MINGW64_CC" && break +done + +test -n "$MINGW64_CC" || as_fn_error $? "no acceptable mingw64 gcc found in \$PATH" "$LINENO" 5 + configure_args=X diff --git a/winsup/utils/configure.ac b/winsup/utils/configure.ac index 63fc55e562..1cad2b9c05 100644 --- a/winsup/utils/configure.ac +++ b/winsup/utils/configure.ac @@ -35,6 +35,11 @@ AC_PROG_INSTALL AC_CHECK_PROGS(MINGW_CXX, ${target_cpu}-w64-mingw32-g++) test -n "$MINGW_CXX" || AC_MSG_ERROR([no acceptable mingw g++ found in \$PATH]) +AC_CHECK_PROGS(MINGW32_CC, i686-w64-mingw32-gcc) +test -n "$MINGW32_CC" || AC_MSG_ERROR([no acceptable mingw32 gcc found in \$PATH]) +AC_CHECK_PROGS(MINGW64_CC, x86_64-w64-mingw32-gcc) +test -n "$MINGW64_CC" || AC_MSG_ERROR([no acceptable mingw64 gcc found in \$PATH]) + AC_EXEEXT AC_CONFIGURE_ARGS AC_CONFIG_FILES([Makefile]) diff --git a/winsup/utils/getprocaddr.c b/winsup/utils/getprocaddr.c new file mode 100644 index 0000000000..54f8e27954 --- /dev/null +++ b/winsup/utils/getprocaddr.c @@ -0,0 +1,146 @@ +#include +#include + +/** + * To determine the address of kernel32!CtrlRoutine, we need to use + * dbghelp.dll. But we want to avoid linking statically to that library because + * the normal operation of cygwin-console-helper.exe (i.e. to allocate a hidden + * Console) does not need it. + * + * Therefore, we declare the SYMBOL_INFOW structure here, load the dbghelp + * library via LoadLibraryExA() and obtain the SymInitialize(), SymFromAddrW() + * and SymCleanup() functions via GetProcAddr(). + */ + +#include + +/* Avoid fprintf(), as it would try to reference '__getreent' */ +static void +output (BOOL error, const char *fmt, ...) +{ + va_list ap; + char buffer[1024]; + + va_start (ap, fmt); + vsnprintf (buffer, sizeof(buffer) - 1, fmt, ap); + buffer[sizeof(buffer) - 1] = '\0'; + va_end (ap); + WriteFile (GetStdHandle(error ? STD_ERROR_HANDLE : STD_OUTPUT_HANDLE), + buffer, strlen (buffer), NULL, NULL); +} + +static WINAPI BOOL +ctrl_handler(DWORD ctrl_type) +{ + unsigned short count; + void *address; + HANDLE process; + PSYMBOL_INFOW info; + DWORD64 displacement; + + count = CaptureStackBackTrace (1l /* skip this function */, + 1l /* return only one trace item */, + &address, NULL); + if (count != 1) + { + output (1, "Could not capture backtrace\n"); + return FALSE; + } + + process = GetCurrentProcess (); + if (!SymInitialize (process, NULL, TRUE)) + { + output (1, "Could not initialize symbols\n"); + return FALSE; + } + + info = (PSYMBOL_INFOW) + malloc (sizeof(*info) + MAX_SYM_NAME * sizeof(wchar_t)); + if (!info) + { + output (1, "Could not allocate symbol info structure\n"); + return FALSE; + } + info->SizeOfStruct = sizeof(*info); + info->MaxNameLen = MAX_SYM_NAME; + + if (!SymFromAddrW (process, (DWORD64)(intptr_t)address, &displacement, info)) + { + output (1, "Could not get symbol info\n"); + SymCleanup(process); + return FALSE; + } + output (0, "%p\n", (void *)(intptr_t)info->Address); + CloseHandle(GetStdHandle(STD_OUTPUT_HANDLE)); + SymCleanup(process); + + exit(0); +} + +int +main (int argc, char **argv) +{ + char *end; + + if (argc < 2) + { + output (1, "Need a function name\n"); + return 1; + } + + if (strcmp(argv[1], "CtrlRoutine")) + { + if (argc > 2) + { + output (1, "Unhandled option: %s\n", argv[2]); + return 1; + } + + HINSTANCE kernel32 = GetModuleHandle ("kernel32"); + if (!kernel32) + return 1; + void *address = (void *) GetProcAddress (kernel32, argv[1]); + if (!address) + return 1; + output (0, "%p\n", address); + return 0; + } + + /* Special-case kernel32!CtrlRoutine */ + if (argc == 3 && !strcmp ("--alloc-console", argv[2])) + { + if (!FreeConsole () && GetLastError () != ERROR_INVALID_PARAMETER) + { + output (1, "Could not detach from current Console: %d\n", + (int)GetLastError()); + return 1; + } + if (!AllocConsole ()) + { + output (1, "Could not allocate a new Console\n"); + return 1; + } + } + else if (argc > 2) + { + output (1, "Unhandled option: %s\n", argv[2]); + return 1; + } + + if (!SetConsoleCtrlHandler (ctrl_handler, TRUE)) + { + output (1, "Could not register Ctrl handler\n"); + return 1; + } + + if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, 0)) + { + output (1, "Could not simulate Ctrl+Break\n"); + return 1; + } + + /* Give the event 1sec time to print out the address */ + Sleep(1000); + return 1; +} + From 4ae93069adfb0c2bfd0ae921cf0d1133e56e8f46 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 17 Apr 2018 11:56:50 +0200 Subject: [PATCH 2/3] squash! Try to handle Ctrl+C on Win32 processes the same way CMD does 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 | 4 +- winsup/cygwin/include/cygwin/exit_process.h | 374 +++++++++++++++----- 2 files changed, 290 insertions(+), 88 deletions(-) diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc index 51a3ed11a9..7e831702e4 100644 --- a/winsup/cygwin/exceptions.cc +++ b/winsup/cygwin/exceptions.cc @@ -1561,8 +1561,8 @@ sigpacket::process () goto done; default: sigproc_printf ("terminating captive process"); - exit_process (ch_spawn, 128 + (sigExeced = si.si_signo), 0); - break; + rc = exit_process_tree (ch_spawn, 128 + (sigExeced = si.si_signo)); + goto done; } } /* Dispatch to the appropriate function. */ diff --git a/winsup/cygwin/include/cygwin/exit_process.h b/winsup/cygwin/include/cygwin/exit_process.h index 6783950ad2..e0981bbe4a 100644 --- a/winsup/cygwin/include/cygwin/exit_process.h +++ b/winsup/cygwin/include/cygwin/exit_process.h @@ -5,14 +5,232 @@ * This file contains functions to terminate a Win32 process, as gently as * possible. * - * If appropriate, we will attempt to generate a console Ctrl event. - * Otherwise we will fall back to terminating the entire process tree. + * 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 +#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 @@ -22,24 +240,51 @@ * .lock files, terminating spawned processes (if any), etc). */ static int -terminate_process(HANDLE process, int exit_code) +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 provided - * terminate callback function + * directly and indirectly spawned subprocesses using the + * exit_one_process() function. */ static int -terminate_process_tree(HANDLE main_process, int exit_code, int (*terminate)(HANDLE, 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; @@ -55,29 +300,36 @@ terminate_process_tree(HANDLE main_process, int exit_code, int (*terminate)(HAND */ for (;;) { - int orig_len = len; - pid_t cyg_pid; - 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--) { - cyg_pid = cygwin_winpid_to_pid(entry.th32ProcessID); + 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) { - kill(cyg_pid, exit_code); - continue; + if (cyg_pid == getpgid (cyg_pid)) + kill(cyg_pid, signo); + break; } - if (pids[i] == entry.th32ProcessID) - break; - if (pids[i] == entry.th32ParentProcessID) - pids[len++] = entry.th32ProcessID; + pids[len++] = entry.th32ProcessID; + break; } } while (len < max_len && Process32Next (snapshot, &entry)); @@ -88,82 +340,32 @@ terminate_process_tree(HANDLE main_process, int exit_code, int (*terminate)(HAND for (i = len - 1; i >= 0; i--) { - HANDLE process = i == 0 ? main_process : - OpenProcess (PROCESS_TERMINATE, FALSE, pids[i]); + HANDLE process; - if (process) + if (!i) + process = main_process; + else { - if (!(*terminate) (process, exit_code)) - ret = -1; - CloseHandle (process); - } - } + 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; - return ret; -} - -/** - * For SIGINT/SIGTERM, call GenerateConsoleCtrlEven(). Otherwise fall back to - * running terminate_process_tree(). - */ -static int -exit_process(HANDLE process, int exit_code, int okay_to_kill_this_process) -{ - DWORD code; - - if (GetExitCodeProcess (process, &code) && code == STILL_ACTIVE) - { - int signal = exit_code & 0x7f; - if (signal == SIGINT || signal == SIGTERM) + if (process && + (!GetExitCodeProcess (process, &code) || code == STILL_ACTIVE)) { -#ifndef __INSIDE_CYGWIN__ - if (!okay_to_kill_this_process) - return -1; - FreeConsole(); - if (!AttachConsole(GetProcessId(process))) - return -1; - if (GenerateConsoleCtrlEvent(signal == SIGINT ? CTRL_C_EVENT : CTRL_BREAK_EVENT, 0)) - return 0; -#else - path_conv helper ("/bin/kill.exe"); - if (helper.exists ()) - { - STARTUPINFOW si = {}; - PROCESS_INFORMATION pi; - size_t len = helper.get_wide_win32_path_len (); - WCHAR cmd[len + (2 * strlen (" -f -32 0xffffffff")) + 1]; - WCHAR title[] = L"kill"; - - helper.get_wide_win32_path (cmd); - __small_swprintf (cmd + len, L" -f -%d %d", signal, (int)GetProcessId(ch_spawn)); - - si.cb = sizeof (si); - si.dwFlags = STARTF_USESHOWWINDOW; - si.wShowWindow = SW_HIDE; - si.lpTitle = title; - - /* Create a new hidden process. Use the two event handles as - argv[1] and argv[2]. */ - BOOL x = CreateProcessW (NULL, cmd, &sec_none_nih, &sec_none_nih, - true, CREATE_NO_WINDOW, NULL, NULL, &si, &pi); - if (x) - { - CloseHandle (pi.hThread); - if (WaitForSingleObject (pi.hProcess, 10000) == WAIT_OBJECT_0) - { - CloseHandle (pi.hProcess); - return 0; - } - CloseHandle (pi.hProcess); - } - } -#endif + if (!exit_one_process (process, exit_code)) + ret = -1; } - - return terminate_process_tree (process, exit_code, terminate_process); + if (process) + CloseHandle (process); } - return -1; + return ret; } #endif From c87a1769878c7f4d5f7348126a5db7d72e184dd3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 22 Apr 2018 23:40:33 +0200 Subject: [PATCH 3/3] fixup! kill: kill Win32 processes more gently --- winsup/utils/kill.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/winsup/utils/kill.cc b/winsup/utils/kill.cc index 911eedb01a..955d4eb46f 100644 --- a/winsup/utils/kill.cc +++ b/winsup/utils/kill.cc @@ -174,7 +174,7 @@ forcekill (int pid, int sig, int wait) if (!wait || WaitForSingleObject (h, 200) != WAIT_OBJECT_0) { HANDLE cur = GetCurrentProcess (), h2; - /* duplicate handle with access rights required for exit_process() */ + /* 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 | @@ -184,7 +184,7 @@ forcekill (int pid, int sig, int wait) CloseHandle(h); h = h2; } - exit_process (h2, 128 + sig, 1); + exit_process_tree (h2, 128 + sig); } CloseHandle (h); }