mingw: terminate child processes in a gentler way

This patch series consists of two patches that have been carried in Git for Windows since 2017 an 2018, respectively.

The problem they work around is a fundamental mismatch between Git's understanding how processes can be terminated and Windows' multi-threading centric world view (where multi-process architectures are quite, quite rare), where processes do not tell other processes to terminate gently (meaning: giving them a chance to run their atexit() handlers).

As such, Git thinks that it can send processes signals to terminate or force-stop ("kill") them. There are no signals in the Unix sense on Windows, though. So we try to emulate them. At present, in vanilla Git that means that we use the TerminateProcess() Win32 API functions, which is most similar to Unix' SIGKILL and is typically frowned upon because it does not allow an orderly shutdown of multi-threaded applications. That's definitely not what Git wants to do: If it wants to terminate a child process, it wants that child process to clean up any .lock files, for example. And therefore it wants to send a SIGTERM.

But the SIGTERM signal does not really have any equivalent on Windows. The closest is to somehow get the target process to call the ExitProcess() Win32 API function. There is a trick that we employ here to do precisely that: we create a remote thread in the target process, and specify the ExitProcess() function as the callee. This works because that function matches the function signature of thread functions enough that we can get away with it, and because the address of that function is identical between processes matching the same CPU architecture. Read: This approach does not work when trying to terminate i686 processes from an x86_64 git.exe. But since it is rare to mix and match processes of different CPU architectures on Windows (certainly in Git scenarios), we kind of resort to this best effort that works often enough to make it worthwhile.

It's a different story for SIGINT: That signal matches most closely what Windows calls a ConsoleCtrlEvent. It is different, though, in that a ConsoleCtrlEvent is not sent to a process, but to a Console, and is handled by all processes that are attached to said Console. In the MSYS2 runtime that provides the POSIX emulation layer required by the Bash distributed with Git for Windows, we work around that by using a similar trick as the SIGTERM/ExitProcess() injection: a thread is injected into the remote process, passing the address of the (undocumented) kernel32!CtrlRoutine. This is quite hacky and requires spawning a separate process to just to figure out the address of said function, which only works in the MSYS2 runtime because it acquires that address once, and then remembers it for the rest of its lifetime. Git also simply has no business emulating a Ctrl+C and instead sends child processes SIGTERM. Therefore, there is no support for sending SIGINT in this patch series. But patch number 2 implements reacting to the emulated SIGINT "sent" by the MSYS2 runtime.

Johannes Schindelin (2):
mingw: kill child processes in a gentler way
mingw: really handle SIGINT

 compat/mingw.c              |  38 +++++++--
 compat/win32/exit-process.h | 165 ++++++++++++++++++++++++++++++++++++
 2 files changed, 195 insertions(+), 8 deletions(-)
 create mode 100644 compat/win32/exit-process.h

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2130%2Fdscho%2Fmingw-kill-gentle-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2130/dscho/mingw-kill-gentle-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2130

gitgitgadget

From: Johannes Schindelin <[email protected]>

The TerminateProcess() function does not actually leave the child processes any chance to perform any cleanup operations. This is bad insofar as Git itself expects its signal handlers to run.

A symptom is e.g. a left-behind .lock file that would not be left behind if the same operation was run, say, on Linux.

To remedy this situation, we use an obscure trick: we inject a thread into the process that needs to be killed and to let that thread run the ExitProcess() function with the desired exit status. Thanks J Wyman for describing this trick.

The advantage is that the ExitProcess() function lets the atexit handlers run. While this is still different from what Git expects (i.e. running a signal handler), in practice Git sets up signal handlers and atexit handlers that call the same code to clean up after itself.

In case that the gentle method to terminate the process failed, we still fall back to calling TerminateProcess(), but in that case we now also make sure that processes spawned by the spawned process are terminated; TerminateProcess() does not give the spawned process a chance to do so itself.

Please note that this change only affects how Git for Windows tries to terminate processes spawned by Git's own executables. Third-party software that *calls* Git and wants to terminate it *still* need to make sure to imitate this gentle method, otherwise this patch will not have any effect.

---
 compat/mingw.c              |  29 +++++--
 compat/win32/exit-process.h | 165 ++++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+), 8 deletions(-)
 create mode 100644 compat/win32/exit-process.h

index 2023c16db6..973049ffe3 100644
@@ -13,6 +13,7 @@
 #include "symlinks.h"
 #include "trace2.h"
 #include "win32.h"
+#include "win32/exit-process.h"
 #include "win32/lazyload.h"
 #include "wrapper.h"
 #include <aclapi.h>

@@ -2208,16 +2209,28 @@ int mingw_execvp(const char *cmd, char *const *argv)
 int mingw_kill(pid_t pid, int sig)
 {
 	if (pid > 0 && sig == SIGTERM) {
-		HANDLE h = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
-
-		if (TerminateProcess(h, -1)) {
+		HANDLE h = OpenProcess(PROCESS_CREATE_THREAD |
+				       PROCESS_QUERY_INFORMATION |
+				       PROCESS_VM_OPERATION | PROCESS_VM_WRITE |
+				       PROCESS_VM_READ | PROCESS_TERMINATE,
+				       FALSE, pid);
+		int ret;
+
+		if (h)
+			ret = exit_process(h, 128 + sig);
+		else {
+			h = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
+			if (!h) {
+				errno = err_win_to_posix(GetLastError());
+				return -1;
+			}
+			ret = terminate_process_tree(h, 128 + sig);
+		}
+		if (ret) {
+			errno = err_win_to_posix(GetLastError());
 			CloseHandle(h);
-			return 0;
 		}
-
-		errno = err_win_to_posix(GetLastError());
-		CloseHandle(h);
-		return -1;
+		return ret;
 	} else if (pid > 0 && sig == 0) {
 		HANDLE h = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);
 		if (h) {

gitgitgadget

From: Johannes Schindelin <[email protected]>

Previously, we did not install any handler for Ctrl+C, but now we really want to because the MSYS2 runtime learned the trick to call the ConsoleCtrlHandler when Ctrl+C was pressed.

With this, hitting Ctrl+C while git log is running will only terminate the Git process, but not the pager. This finally matches the behavior on Linux and on macOS.

---
 compat/mingw.c | 9 +++++++++
 1 file changed, 9 insertions(+)

index 973049ffe3..f2b6c51f98 100644
@@ -3580,7 +3580,14 @@ static void adjust_symlink_flags(void)
 		symlink_file_flags |= 2;
 		symlink_directory_flags |= 2;
 	}
+}
 
+static BOOL WINAPI handle_ctrl_c(DWORD ctrl_type)
+{
+	if (ctrl_type != CTRL_C_EVENT)
+		return FALSE; /* we did not handle this */
+	mingw_raise(SIGINT);
+	return TRUE; /* we did handle this */
 }
 
 #ifdef _MSC_VER

@@ -3617,6 +3624,8 @@ int wmain(int argc, const wchar_t **wargv)
 #endif
 #endif
 
+	SetConsoleCtrlHandler(handle_ctrl_c, TRUE);
+
 	maybe_redirect_std_handles();
 	adjust_symlink_flags();
 

gitgitgadget