Skip to content

Ruby executed via /usr/bin/env cannot be interrupted #1648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task done
orgads opened this issue Apr 23, 2018 · 12 comments
Closed
1 task done

Ruby executed via /usr/bin/env cannot be interrupted #1648

orgads opened this issue Apr 23, 2018 · 12 comments
Assignees

Comments

@orgads
Copy link

orgads commented Apr 23, 2018

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options

git version 2.17.0.windows.1.26.g82f1b61609
cpu: x86_64
built from commit: 82f1b6160988ab0a61edebef6ed1c3048b66a3a4
sizeof-long: 4
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd.exe /c ver

Microsoft Windows [Version 6.1.7601]
  • What options did you set as part of the installation? Or did you choose the
    defaults?
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Editor Option: VIM
Path Option: Cmd
SSH Option: OpenSSH
CURL Option: WinSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Enabled

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

Bash

ruby --version:

ruby 2.4.4p296 (2018-03-28 revision 63013) [x64-mingw32]
/usr/bin/env ruby -e 'sleep 10' # Cannot be interrupted
<Ctrl-C>
ruby -e 'sleep 10' # Can be interrupted
<Ctrl-C>
-e:1:in `sleep': Interrupt
        from -e:1:in `<main>'
  • What did you expect to occur after running these commands?

It should stop the process

  • What actually happened instead?

It doesn't.

@orgads
Copy link
Author

orgads commented Apr 23, 2018

I also had this issue with previous Git versions (at least since 2.16, possibly before). Not sure if this is Ruby's or MSYS's fault.

@dscho
Copy link
Member

dscho commented Apr 23, 2018

@orgads I am glad you opened this ticket. I am glad because I know a little bit about you from previous experience, in particular that you are quite capable, once you put yourself behind fixing an issue.

In this case, I would like to suggest to instrument the code in exit_one_process() (and then rebuild and test) to see what the address is in the different switch cases, where it falls through, and what the error conditions are.

Of course, the culprit may very well be that this code (whose intention is to avoid killing MSYS2 processes using the method intended to kill non-MSYS2 processes) does not do what it is supposed to do: https://github.com/git-for-windows/msys2-runtime/blob/fa84a1f06b83b54847e6505df2bdb6db1860f47f/winsup/cygwin/include/cygwin/exit_process.h#L325-L330

@orgads
Copy link
Author

orgads commented Apr 24, 2018

Added some debugging info. Patch here: https://gist.github.com/orgads/460270b84d9b0849b26d65ca15bcd302

$ ruby -e 'sleep 5'
exit_process_tree. pid: 8340, signo: 2
match pids[0] == 8340
signo: 2
address: 0x77614B10
injecting: 0. thread_id: 4704, thread: 0x164
-e:1:in `sleep': Interrupt
        from -e:1:in `<main>'
$ env ruby -e 'sleep 5'
exit_process_tree. pid: 10772, signo: 2
match pids[0] == 10772
signo: 2
address: 0x77614B10
injecting: 0. thread_id: 5496, thread: 0xF8
<Not terminated. Another Ctrl-C>
exit_process_tree. pid: 0, signo: 2
match pids[0] == 0
match parent pids[0] == 0
cyg_pid: -1
Added pids[1] = 4
match parent pids[1] == 4
cyg_pid: -1
Added pids[2] = 376
match parent pids[2] == 376
cyg_pid: -1
Added pids[3] = 932
match pids[0] == 0
match pids[1] == 4
match pids[2] == 376
match pids[3] == 932
signo: 2
address: 0x77614B10
injecting: 0. thread_id: 1, thread: 0x0
address on SIGTERM: 0x77702840
injecting: 130. thread_id: 1, thread: 0x0
signo: 2
address: 0x77614B10
injecting: 0. thread_id: 1, thread: 0x0
address on SIGTERM: 0x77702840
injecting: 130. thread_id: 1, thread: 0x0
signo: 2
address: 0x0
address on SIGTERM: 0x0
signo: 2
address: 0x0
address on SIGTERM: 0x0

@orgads
Copy link
Author

orgads commented Apr 24, 2018

Some more observations: When running through env, exit_process_tree receives the pid of ruby (as main_process), and not of env. Still, none of them receives the signal.

@dscho
Copy link
Member

dscho commented Apr 24, 2018

Probably ruby has env as parent process, and I vaguely seem to remember that there was some code that left SIGINT handling to the process group leader. Maybe this is what is at play here?

clicketyclick

https://github.com/git-for-windows/msys2-runtime/blob/fa84a1f06b83b54847e6505df2bdb6db1860f47f/winsup/cygwin/exceptions.cc#L1147-L1165

But that only happens in the ctrl_c_handler() function which, as far as I can see, is only installed via SetConsoleCtrlHandler() and not called directly.

@dscho
Copy link
Member

dscho commented Apr 24, 2018

What is this all about?

@orgads
Copy link
Author

orgads commented Apr 24, 2018

Ruby scripts typically use the shebang #!/usr/bin/env ruby, and not being able to interrupt them is, well, unfortunate.

I bisected the msys2-runtime directory in MSYS2-packages, and found that the first bad commit is git-for-windows/MSYS2-packages@ac09de9.

@orgads
Copy link
Author

orgads commented Apr 24, 2018

I added some logs in ctrl_c_handler. It is never called for neither of the executions.

@dscho
Copy link
Member

dscho commented Apr 24, 2018

I bisected the msys2-runtime directory in MSYS2-packages, and found that the first bad commit is git-for-windows/MSYS2-packages@ac09de9.

Hmm. That one switches to calling GenerateConsoleCtrlEvent()...

I also tried this with a very simple C executable that basically just registers a ConsoleCtrlHandler() and then sleeps for ten seconds.

And while I could send a CTRL_BREAK_EVENT successfully (and it was caught correctly), the CTRL_C_EVENT seems to be simply ignored 😦

@orgads
Copy link
Author

orgads commented May 29, 2018

Any progress with this? Anything I can do to help?

@dscho
Copy link
Member

dscho commented May 30, 2018

@orgads glad you asked: git-for-windows/msys2-runtime@master...dscho:ctrl-c-harder

This needs testing, especially in a 32-bit runtime that wants to interrupt 32-bit and 64-bit processes, and it needs to learn to skip processes where IsProcessCritical() returns TRUE (but we have to load that symbol dynamically because it is available only in Windows 8.1 and later).

dscho added a commit to dscho/msys2-runtime that referenced this issue May 30, 2018
It seems that the "signal" associated with a Ctrl+C (i.e. a
CTRL_C_EVENT) is sometimes not passed through to the registered handler.

This symptom occurs e.g. when spawning a non-MSYS2 process in Bash
through env.exe, such as is commonly the case when starting ruby scripts
via the shebang line `#!/usr/bin/env ruby` (which usually calls
/mingw64/bin/ruby.exe). The unexpected behavior is that a CtrlRoutine
thread can be executed successfully but does not result in the process'
handler to be called, let alone in the process being terminated.

Even more curious is that a CTRL_BREAK_EVENT is delivered without
problems.

A rather intense few days of quality time with GDB and DuckDuckGo turned
into the following analysis of the issue:

It turns out that after calling EnterCriticalSection(), CtrlRoutine()
asks whether it has been passed 0 (CTRL_C_EVENT) as a parameter and in
that case, jumps to conditional code (hex addresses removed from GDB's
disassembly to save on space):

<KERNELBASE!CtrlRoutine+91>:  callq  *0x17c9af(%rip)
<KERNELBASE!CtrlRoutine+97>:  andl   $0x0,0x24(%rsp)
<KERNELBASE!CtrlRoutine+102>: test   %ebx,%ebx
<KERNELBASE!CtrlRoutine+104>: je     <KERNELBASE!CtrlRoutine+163>

That conditional code reads thusly:

<KERNELBASE!CtrlRoutine+163>: mov    %gs:0x60,%rax
<KERNELBASE!CtrlRoutine+172>: mov    0x20(%rax),%rcx
<KERNELBASE!CtrlRoutine+176>: testb  $0x1,0x18(%rcx)
<KERNELBASE!CtrlRoutine+180>: jne    <KERNELBASE!CtrlRoutine+216>
<KERNELBASE!CtrlRoutine+182>: jmp    <KERNELBASE!CtrlRoutine+106>

This code looks at %gs:0x60, which according to
https://en.wikipedia.org/wiki/Win32_Thread_Information_Block is the PEB
(Process Environment Block). Then it accesses the field of the PEB at
offset 0x20, which is the ProcessParameters field according to
https://msdn.microsoft.com/en-us/library/windows/desktop/aa813706.aspx

These process parameters are described here:
http://undocumented.ntinternals.net/UserMode/Structures/RTL_USER_PROCESS_PARAMETERS.html

Sadly, it is unclear what the field at offset 0x18 (named
`ConsoleFlags`) does, but from the disassembly it is clear that if it
has its least significant bit set, CtrlRoutine() simply cleans up and
exits. The handler(s) only get a chance to run when that bit is 0. (Note
that ReactOS expects *all* of ConsoleFlags to be different from 1:
https://github.com/reactos/reactos/blob/ae9702fce/dll/win32/kernel32/client/console/console.c#L169)

Note: When a 64-bit process asks for the PEB of a 32-bit process, it
does receive a copy of *a 64-bit version* of it. This 64-bit version
points to a copy of the USER_PROCESS_INFORMATION struct that is
compatible with 64-bit, but writing to it will not have any effect! We
instead need to access the 32-bit PEB, which has a pointer to the 32-bit
version of the process information, where we modify the ConsoleFlags. We
are using a special trick to obtain the 32-bit PEB inspired by
https://stackoverflow.com/a/23842609: asking NtQueryInformation() for
the ProcessWow64Information will return the 64-bit address of the 32-bit
PEB, if there is any (indicating that the process in question is a WoW
processes i.e. 32-bit processes running on 64-bit Windows).

This closes git-for-windows/git#1648 and
git-for-windows/git#1470

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Oct 15, 2021

Closing this as stale.

@dscho dscho closed this as completed Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants