Skip to content

Commit f4fda0f

Browse files
committed
msys2-runtime: revamp Ctrl+C handling yet again
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 git-for-windows/msys2-runtime@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: git-for-windows/msys2-runtime@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: git-for-windows/msys2-runtime@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: git-for-windows/msys2-runtime@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]>
1 parent 3c39a4f commit f4fda0f

12 files changed

+1036
-82
lines changed

msys2-runtime/0001-Add-MSYS-triplets.patch

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,43 +4,43 @@ Date: Tue, 1 Mar 2016 08:51:16 +0300
44
Subject: [PATCH 01/N] Add MSYS triplets.
55

66
---
7-
compile | 4 +-
8-
config.guess | 3 ++
9-
config.rpath | 8 ++--
10-
config/dfp.m4 | 3 +-
11-
config/elf.m4 | 2 +-
12-
config/lthostflags.m4 | 2 +-
13-
config/mmap.m4 | 4 +-
14-
config/picflag.m4 | 2 +
15-
config/tcl.m4 | 4 +-
16-
configure | 22 ++++-----
17-
configure.ac | 20 ++++-----
18-
libtool.m4 | 36 ++++++++-------
19-
ltmain.sh | 52 +++++++++++-----------
20-
ltoptions.m4 | 2 +-
21-
newlib/configure | 30 ++++++++-----
22-
newlib/configure.host | 8 ++--
23-
newlib/iconvdata/configure | 30 ++++++++-----
24-
newlib/libc/configure | 30 ++++++++-----
25-
newlib/libc/machine/configure | 30 ++++++++-----
26-
newlib/libc/machine/i386/configure | 30 ++++++++-----
27-
newlib/libc/sys/configure | 30 ++++++++-----
28-
newlib/libc/sys/linux/configure | 30 ++++++++-----
29-
newlib/libc/sys/linux/linuxthreads/configure | 30 ++++++++-----
30-
.../libc/sys/linux/linuxthreads/machine/configure | 30 ++++++++-----
31-
.../sys/linux/linuxthreads/machine/i386/configure | 30 ++++++++-----
32-
newlib/libc/sys/linux/machine/configure | 30 ++++++++-----
33-
newlib/libc/sys/linux/machine/i386/configure | 30 ++++++++-----
34-
newlib/libm/configure | 30 ++++++++-----
35-
newlib/libm/machine/configure | 30 ++++++++-----
36-
newlib/libm/machine/i386/configure | 30 ++++++++-----
37-
winsup/config.guess | 3 ++
38-
winsup/configure | 4 +-
39-
winsup/configure.ac | 2 +-
40-
winsup/cygserver/configure | 2 +-
41-
winsup/cygserver/configure.ac | 2 +-
42-
winsup/cygwin/configure | 2 +-
43-
winsup/cygwin/configure.ac | 2 +-
7+
compile | 4 +-
8+
config.guess | 3 ++
9+
config.rpath | 8 +--
10+
config/dfp.m4 | 3 +-
11+
config/elf.m4 | 2 +-
12+
config/lthostflags.m4 | 2 +-
13+
config/mmap.m4 | 4 +-
14+
config/picflag.m4 | 2 +
15+
config/tcl.m4 | 4 +-
16+
configure | 22 ++++----
17+
configure.ac | 20 +++----
18+
libtool.m4 | 36 +++++++------
19+
ltmain.sh | 52 +++++++++----------
20+
ltoptions.m4 | 2 +-
21+
newlib/configure | 30 ++++++-----
22+
newlib/configure.host | 8 +--
23+
newlib/iconvdata/configure | 30 ++++++-----
24+
newlib/libc/configure | 30 ++++++-----
25+
newlib/libc/machine/configure | 30 ++++++-----
26+
newlib/libc/machine/i386/configure | 30 ++++++-----
27+
newlib/libc/sys/configure | 30 ++++++-----
28+
newlib/libc/sys/linux/configure | 30 ++++++-----
29+
newlib/libc/sys/linux/linuxthreads/configure | 30 ++++++-----
30+
.../sys/linux/linuxthreads/machine/configure | 30 ++++++-----
31+
.../linux/linuxthreads/machine/i386/configure | 30 ++++++-----
32+
newlib/libc/sys/linux/machine/configure | 30 ++++++-----
33+
newlib/libc/sys/linux/machine/i386/configure | 30 ++++++-----
34+
newlib/libm/configure | 30 ++++++-----
35+
newlib/libm/machine/configure | 30 ++++++-----
36+
newlib/libm/machine/i386/configure | 30 ++++++-----
37+
winsup/config.guess | 3 ++
38+
winsup/configure | 4 +-
39+
winsup/configure.ac | 2 +-
40+
winsup/cygserver/configure | 2 +-
41+
winsup/cygserver/configure.ac | 2 +-
42+
winsup/cygwin/configure | 2 +-
43+
winsup/cygwin/configure.ac | 2 +-
4444
37 files changed, 372 insertions(+), 267 deletions(-)
4545

4646
diff --git a/compile b/compile

msys2-runtime/0002-Rename-DLL-from-cygwin-to-msys.patch

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,59 +5,59 @@ Subject: [PATCH 02/N] Rename DLL from cygwin to msys
55

66
---
77
winsup/Makefile.in | 4 +--
8-
winsup/cygserver/Makefile.in | 6 ++---
8+
winsup/cygserver/Makefile.in | 6 ++--
99
winsup/cygserver/transport_pipes.h | 4 +++
10-
winsup/cygwin/Makefile.in | 38 ++++++++++++++-------------
10+
winsup/cygwin/Makefile.in | 38 ++++++++++----------
1111
winsup/cygwin/common.din | 4 +--
1212
winsup/cygwin/configure | 4 +--
1313
winsup/cygwin/configure.ac | 4 +--
14-
winsup/cygwin/crt0.c | 8 ++++++
14+
winsup/cygwin/crt0.c | 8 +++++
1515
winsup/cygwin/cyglsa.h | 4 +++
1616
winsup/cygwin/cygserver_setpwd.h | 4 +++
1717
winsup/cygwin/cygthread.cc | 2 +-
18-
winsup/cygwin/cygwin.sc.in | 21 ++++++++++-----
19-
winsup/cygwin/dcrt0.cc | 20 ++++++++------
18+
winsup/cygwin/cygwin.sc.in | 21 +++++++----
19+
winsup/cygwin/dcrt0.cc | 20 ++++++-----
2020
winsup/cygwin/devices.cc | 2 +-
2121
winsup/cygwin/devices.in | 2 +-
22-
winsup/cygwin/dlfcn.cc | 5 ++++
23-
winsup/cygwin/dll_init.cc | 8 ++++++
24-
winsup/cygwin/dtable.cc | 6 +++++
22+
winsup/cygwin/dlfcn.cc | 5 +++
23+
winsup/cygwin/dll_init.cc | 8 +++++
24+
winsup/cygwin/dtable.cc | 6 ++++
2525
winsup/cygwin/exceptions.cc | 4 +--
26-
winsup/cygwin/fhandler_tty.cc | 12 +++++++++
26+
winsup/cygwin/fhandler_tty.cc | 12 +++++++
2727
winsup/cygwin/fork.cc | 2 +-
2828
winsup/cygwin/hookapi.cc | 4 +++
29-
winsup/cygwin/i686.din | 6 ++---
30-
winsup/cygwin/include/cygwin/cygwin_dll.h | 14 +++++-----
31-
winsup/cygwin/include/cygwin/version.h | 8 ++++++
29+
winsup/cygwin/i686.din | 6 ++--
30+
winsup/cygwin/include/cygwin/cygwin_dll.h | 14 ++++----
31+
winsup/cygwin/include/cygwin/version.h | 8 +++++
3232
winsup/cygwin/lib/_cygwin_crt0_common.cc | 4 +++
3333
winsup/cygwin/lib/crt0.h | 4 +++
34-
winsup/cygwin/lib/cygwin_attach_dll.c | 8 ++++++
35-
winsup/cygwin/lib/cygwin_crt0.c | 8 ++++++
36-
winsup/cygwin/mkvers.sh | 6 ++---
34+
winsup/cygwin/lib/cygwin_attach_dll.c | 8 +++++
35+
winsup/cygwin/lib/cygwin_crt0.c | 8 +++++
36+
winsup/cygwin/mkvers.sh | 6 ++--
3737
winsup/cygwin/pinfo.cc | 4 +--
3838
winsup/cygwin/pipe.cc | 4 +++
3939
winsup/cygwin/pseudo-reloc.cc | 2 +-
40-
winsup/cygwin/sec_auth.cc | 10 +++----
40+
winsup/cygwin/sec_auth.cc | 10 +++---
4141
winsup/cygwin/syscalls.cc | 4 +--
4242
winsup/cygwin/syslog.cc | 4 +++
4343
winsup/cygwin/winsup.h | 4 +++
4444
winsup/cygwin/winver.rc | 2 +-
4545
winsup/cygwin/x86_64.din | 2 +-
4646
winsup/lsaauth/cyglsa.c | 2 +-
4747
winsup/testsuite/Makefile.in | 2 +-
48-
winsup/testsuite/config/default.exp | 8 +++---
49-
winsup/testsuite/cygrun.c | 6 ++---
50-
winsup/testsuite/winsup.api/cygload.cc | 12 ++++-----
48+
winsup/testsuite/config/default.exp | 8 ++---
49+
winsup/testsuite/cygrun.c | 6 ++--
50+
winsup/testsuite/winsup.api/cygload.cc | 12 +++----
5151
winsup/testsuite/winsup.api/cygload.exp | 2 +-
5252
winsup/testsuite/winsup.api/cygload.h | 2 +-
5353
winsup/testsuite/winsup.api/winsup.exp | 2 +-
54-
winsup/utils/Makefile.in | 8 +++---
55-
winsup/utils/cygcheck.cc | 43 +++++++++++++++----------------
54+
winsup/utils/Makefile.in | 8 ++---
55+
winsup/utils/cygcheck.cc | 43 +++++++++++------------
5656
winsup/utils/ldd.cc | 2 +-
57-
winsup/utils/loadlib.h | 6 ++---
58-
winsup/utils/path.cc | 12 ++++-----
59-
winsup/utils/ssp.c | 8 +++---
60-
winsup/utils/strace.cc | 10 +++----
57+
winsup/utils/loadlib.h | 6 ++--
58+
winsup/utils/path.cc | 12 +++----
59+
winsup/utils/ssp.c | 8 ++---
60+
winsup/utils/strace.cc | 10 +++---
6161
54 files changed, 250 insertions(+), 137 deletions(-)
6262

6363
diff --git a/winsup/Makefile.in b/winsup/Makefile.in

msys2-runtime/0003-Add-functionality-for-converting-UNIX-paths-in-argum.patch

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ Subject: [PATCH 03/N] Add functionality for converting UNIX paths in
1111
winsup/cygwin/environ.h | 2 +-
1212
winsup/cygwin/external.cc | 2 +-
1313
winsup/cygwin/include/sys/cygwin.h | 6 +
14-
winsup/cygwin/msys2_path_conv.cc | 630 +++++++++++++++++++++++++++++++++++++
15-
winsup/cygwin/msys2_path_conv.h | 146 +++++++++
16-
winsup/cygwin/path.cc | 64 ++++
14+
winsup/cygwin/msys2_path_conv.cc | 630 +++++++++++++++++++++++++++++
15+
winsup/cygwin/msys2_path_conv.h | 146 +++++++
16+
winsup/cygwin/path.cc | 64 +++
1717
winsup/cygwin/spawn.cc | 48 ++-
1818
winsup/cygwin/winf.h | 4 +
1919
10 files changed, 910 insertions(+), 9 deletions(-)

msys2-runtime/0004-Add-functionality-for-changing-OS-name-via-MSYSTEM-e.patch

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ Subject: [PATCH 04/N] Add functionality for changing OS name via MSYSTEM
66

77
---
88
winsup/cygserver/cygserver-config | 4 ++--
9-
winsup/cygwin/environ.cc | 36 ++++++++++++++++++++++++++++++++----
9+
winsup/cygwin/environ.cc | 36 +++++++++++++++++++++++++----
1010
winsup/cygwin/include/sys/utsname.h | 2 +-
11-
winsup/cygwin/uname.cc | 6 ++++++
11+
winsup/cygwin/uname.cc | 6 +++++
1212
4 files changed, 41 insertions(+), 7 deletions(-)
1313

1414
diff --git a/winsup/cygserver/cygserver-config b/winsup/cygserver/cygserver-config

msys2-runtime/0005-Move-root-to-usr.-Change-sorting-mount-points.-Do-no.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Subject: [PATCH 05/N] - Move root to /usr. - Change sorting mount points. -
88
---
99
winsup/cygwin/cygheap.cc | 12 ++-
1010
winsup/cygwin/globals.cc | 2 +-
11-
winsup/cygwin/mount.cc | 189 ++++++++++++++++++++++++++++++++++++++++-------
11+
winsup/cygwin/mount.cc | 189 +++++++++++++++++++++++++++++++++------
1212
winsup/cygwin/mount.h | 3 +-
1313
winsup/cygwin/uinfo.cc | 2 +-
1414
5 files changed, 177 insertions(+), 31 deletions(-)

msys2-runtime/0006-Do-not-create-cygwin-symlinks.-Instead-use-deep-copy.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Subject: [PATCH 06/N] Do not create cygwin symlinks. Instead use deep copy of
55
files/folders.
66

77
---
8-
winsup/cygwin/path.cc | 147 ++++++++++++++++++++++++++++++++++++++++++++++++++
8+
winsup/cygwin/path.cc | 147 ++++++++++++++++++++++++++++++++++++++++++
99
1 file changed, 147 insertions(+)
1010

1111
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc

msys2-runtime/0041-Try-to-handle-Ctrl-C-on-Win32-processes-the-same-way.patch

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ already killed appropriately upon Ctrl+C.
4848

4949
Signed-off-by: Johannes Schindelin <[email protected]>
5050
---
51-
winsup/cygwin/exceptions.cc | 20 +++-
52-
winsup/cygwin/include/cygwin/exit_process.h | 169 ++++++++++++++++++++++++++++
51+
winsup/cygwin/exceptions.cc | 20 ++-
52+
winsup/cygwin/include/cygwin/exit_process.h | 169 ++++++++++++++++++++
5353
winsup/cygwin/include/cygwin/signal.h | 1 +
5454
3 files changed, 188 insertions(+), 2 deletions(-)
5555
create mode 100644 winsup/cygwin/include/cygwin/exit_process.h

msys2-runtime/0042-Allow-overriding-the-home-directory-via-the-HOME-var.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ strategy is activated via the `env` keyword in the `db_home` line in
2020
Signed-off-by: Johannes Schindelin <[email protected]>
2121
---
2222
winsup/cygwin/cygheap.h | 3 ++-
23-
winsup/cygwin/uinfo.cc | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
23+
winsup/cygwin/uinfo.cc | 49 +++++++++++++++++++++++++++++++++++++++++
2424
2 files changed, 51 insertions(+), 1 deletion(-)
2525

2626
diff --git a/winsup/cygwin/cygheap.h b/winsup/cygwin/cygheap.h

0 commit comments

Comments
 (0)