Skip to content
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

kill() / signal() doesn't really work. #176

Open
hyc opened this issue Nov 5, 2023 · 12 comments
Open

kill() / signal() doesn't really work. #176

hyc opened this issue Nov 5, 2023 · 12 comments

Comments

@hyc
Copy link

hyc commented Nov 5, 2023

Currently MSYS isn't able to send signals to ordinary Win32 programs. A long time ago I wrote a patch for MSYS1 to accomplish this. https://sourceforge.net/p/mingw/mailman/message/10716023/

It would need refreshing to handle 64bit and 32bit binaries but otherwise should still work. Is there any interest in supporting this?

@lazka
Copy link
Member

lazka commented Nov 5, 2023

Something like this was implemented in #31 if I'm not mistaken.

@hyc
Copy link
Author

hyc commented Nov 5, 2023

That patch only fixes Ctrl-C handling in a console window. The patch I linked to above handles SIGINT/SIGQUIT/SIGKILL/SIGTERM and works even if the target process isn't attached to a terminal.

@lazka
Copy link
Member

lazka commented Nov 5, 2023

That patch only fixes Ctrl-C handling in a console window. The patch I linked to above handles SIGINT/SIGQUIT/SIGKILL/SIGTERM and works even if the target process isn't attached to a terminal.

The patch I linked mentioned them all, and it was written before we used conpty, so also without a attached console.

I don't know much about this, but I think we need an example that you want to work, but currently doesn't, to see if anything is missing.

@hyc
Copy link
Author

hyc commented Nov 5, 2023

I see, didn't read the entire patch thoroughly before. For the moment the only problem I seem to be having is that SIGTERM just calls ExitProcess() so it bypasses any signal handlers.

@lazka
Copy link
Member

lazka commented Nov 5, 2023

Just wondering, is there any Windows software using SIGTERM handlers? And if yes, why?

@sskras
Copy link

sskras commented Nov 5, 2023

@lazka, maybe that MSDN page could shed some light:

The SIGILL and SIGTERM signals aren't generated under Windows. They're included for ANSI compatibility. Therefore, you can set signal handlers for these signals by using signal, and you can also explicitly generate these signals by calling raise.

OTOH there might be some F/LOSS that got ported to Windows + CRT and are historically still trying to handle these two.

@hyc
Copy link
Author

hyc commented Nov 5, 2023

OTOH there might be some F/LOSS that got ported to Windows + CRT and are historically still trying to handle these two.

Exactly. SIGTERM is also the default generated by the kill command.

@lazka
Copy link
Member

lazka commented Nov 5, 2023

OTOH there might be some F/LOSS that got ported to Windows + CRT and are historically still trying to handle these two.

This seems like a bug to me, or potentially problematic if code is executed then which is not meant for that platform.

@sskras
Copy link

sskras commented Nov 5, 2023

@lazka, that was a hypothetical case, and I didn't generated it whole.

The part I omitted might be a guarding thread/process that logs the stack traces of the remaining threads/child processes, and then sends SIGTERM (in platform-dependent ways) to them.

Of course, having a working example would make things clearer :)

@dscho
Copy link
Collaborator

dscho commented Nov 13, 2023

That patch only fixes Ctrl-C handling in a console window.

That is distinctly untrue. That patch is not even necessary in a Console window. That patch was specifically to get SIGINT handlers working in MinTTY, without any Console, not even a pseudo console.

TBH I consider #31 to have addressed this ticket before it was opened 😃

@hyc
Copy link
Author

hyc commented Nov 13, 2023

TBH I consider #31 to have addressed this ticket before it was opened 😃

Mostly. There are still a few signals that the CRT supports that aren't addressed in #31, so the remaining question is if anyone else cares enough about those missing signals. SIGTERM is most significant to me but others might be useful once in a while.

@dscho
Copy link
Collaborator

dscho commented Nov 13, 2023

It has been pointed out that the "missing signals" are not produced by Windows. I see very, very little point, then, to produce them in the MSYS2 runtime.

The only reason to handle SIGTERM in a special way is to give the atexit() handlers a chance to run, which the existing code (as of #31) does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants