Merge branch 'ctrl-c-with-CtrlRoutine'

This thing again...

Background: when you hit Ctrl+C on Linux or macOS, a signal (SIGINT) is
sent to the foreground process and its child processes. This signal can
be intercepted by installing a signal handler for this specific signal.
On Windows, there is no precise equivalent for this system.

Instead, the Ctrl+C is translated by the current ConHost (i.e. the
container running the Console processes) to a ConsoleCtrl event that is
sent to all processes attached to that Console. If any of these
processes installed a handler via SetConsoleCtrlHandler(), they can
intercept that event (and avoid exiting or doing some cleanup work).

On Linux and macOS (and every Unix flavor, really), processes can also
be killed via the `kill` executable, which really just sends a signal to
the process, typically SIGTERM. Processes can intercept that signal,
too. To force processes to terminate, without giving them any chance to
prevent that, SIGKILL can be sent. There is no equivalent for SIGTERM on
Windows. To emulate SIGKILL on Windows, TerminateProcess() can be used,
but it only kills one process (unlike SIGKILL, which is sent also to the
child processes).

In Git for Windows, we struggled with emulating SIGINT, SIGTERM and
SIGKILL handling essentially since the beginning of the efforts to port
Git to Windows.

At least the SIGINT part of the problem becomes a lot worse when using a
terminal window other than cmd.exe's: as long as using cmd.exe (AKA
"ConHost"), Ctrl+C is handled entirely outside of our code. But with the
big jump from v1.x to v2.x, Git for Windows not only switched from MSys
to MSYS2, but also started using MinTTY as the default terminal window,
which uses the MSYS2 runtime-provided pseudo terminals (inherited from
Cygwin thanks to the MSYS2 runtime being a "friendly fork" of Cygwin).
When Ctrl+C is pressed in MinTTY, all of the signaling has to be done by
our code.

The original code to handle Ctrl+C comes straight from Cygwin. It simply
ignores the entire conundrum for non-Cygwin processes and simply calls
TerminateProcess() on them, leaving spawned child processes running.

The first attempt at fixing "the Ctrl+C problem" (with the symptom that
interrupting `git clone ...` would not stop the actual download of the
Git objects that was still running in a child process) was
c4ba4e3357f. It
simply enumerated all the processes' process IDs and parent process IDs
and extracted the tree of (possibly transitive) child processes of the
process to kill, then called TerminateProcess() on them.

This solved the problem with interrupting `git clone`, but it did not
address the problem that Git typically wants to "clean up" when being
interrupted. In particular, Git installs atexit() and signal handlers to
remove .lock files. The most common symptom was that a stale
.git/index.lock file was still present after interrupting a Git process.

Based on the idea presented in Dr Dobb's Journal in the article "A Safer
Alternative to TerminateProcess()" by Andrew Tucker (July 1, 1999)
http://www.drdobbs.com/a-safer-alternative-to-terminateprocess/184416547
we changed our handling to inject a remote thread calling ExitProcess()
first, and fall back to TerminateProcess() the process tree instead:
e9cb332976c

This change was a little misguided in hindsight, as it only called
TerminateProcess() on the process tree, but expected the atexit()
handler of Git to take care of the child processes when killing the
process via the remote ExitProcess() method.

Therefore, we changed the strategy once again, to inject ExitProcess()
threads into the child processes of the process to kill, too:
53e5c0313e1

(That commit also tries to handle Cygwin process among the child
processes by sending Cygwin signals, but unfortunately that part of the
commit was buggy.)

This worked well for Git processes. However, Git Bash is used in all
kinds of circumstances, including launching Maven, or node.js scripts
that want to intercept SIGINT. Naturally, these callees have no idea
that Git for Windows injects an ExitProcess() with exit code 130
(corresponding to 0x100 + SIGINT). Therefore, they never "got" the
signal.

So what is it that happens when ConHost generates a ConsoleCtrl event?
This question was asked and answered in the excellent blog post at:
http://stanislavs.org/stopping-command-line-applications-programatically-with-ctrl-c-events-from-net/#comment-2880

Essentially, the same happens as what we did with ExitProcess(): a
remote thread gets injected, with the event type as parameter. Of course
it is not ExitProcess() that is called, but CtrlRoutine(). This function
lives in kernel32.dll, too, but it is not exported, i.e.
GetProcAddress() won't find it. The trick proposed in the blog post (to
send a test ConsoleCtrl event to the process itself, using a special
handler that then inspects the stack trace to figure out the address of
the caller) does not work for us, however: it would send a
CTRL_BREAK_EVENT to *all* processes attached to the same Console,
essentially killing MinTTY.

But could we make this still work somehow? Yes, we could. We came up
with yet another trick up our sleeves: instead of determining the
address of kernel32!CtrlRoutine in our own process, we spawn a new one,
with a new Console, to avoid killing MinTTY. To do that, we need a
helper .exe, of course, which we put into /usr/libexec/. If this helper
is not found, we fall back to the previous methods of injecting
ExitProcess() or calling TerminateProcess().

This method (to spawn a helper .exe) has a further incidental benefit:
by compiling 32-bit *and* 64-bit helpers and providing them as
getprocaddr32.exe and getprocaddr64.exe, we can now also handle 32-bit
processes in a 64-bit Git for Windows. Sadly not vice versa: calling
CreateRemoteThread() on a 64-bit process from a 32-bit process seems to
fail all the time (and require a lot of assembly hackery to fix that I
am not really willing to include in Git for Windows' MSYS2 runtime).

The current method was implemented in this commit:
ca6188a7520

This is the hopeful final fix for
git-for-windows/git#1491,
git-for-windows/git#1470,
git-for-windows/git#1248,
git-for-windows/git#1239,
git-for-windows/git#227,
git-for-windows/git#1553,
nodejs/node#16103, and plenty other tickets
that petered out mostly due to a willingness of community members to
leave all the hard work to a single, already overworked person.

This fix also partially helps
git-for-windows/git#1629 (only partially
because the user wanted to quit the pager using Ctrl+C, which is not the
intended consequence of a Ctrl+C: it should stop the Git process, but
not the pager).

Signed-off-by: Johannes Schindelin <[email protected]>

Author: dscho

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)

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

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])

winsup/utils/getprocaddr.c

@@ -0,0 +1,146 @@
+#include <stdio.h>
+#include <windows.h>
+
+/**
+ * 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 <dbghelp.h>
+
+/* 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;
+}
+