-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Mono does not emit ProcessExit event on SIGTERM. #81093
Comments
Hmm it feels more like a dotnet/runtime issue to me given this is a difference between Mono and CoreCLR. Should I move the issue? |
I was not sure where to create it. It does look like the ASP.NET app responds to SIGTERM appropriately. |
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
@lambdageek do you know which area we should assign this to? |
@akoeplinger not sure. AppModel, maybe? Cc @vitek-karas It will probably come back to mono, but it's not obvious to me why the behavior difference. |
It is most likely an issue with Mono-specific SIGTERM handler. |
Potential duplicate of #31656 |
I've debugged this issue. Mono doesn't install a signal handler for CoreCLR installs it here:
This event is used by the cli to reap the child processes using its ProcessReaper class. A simpler reproducer for the issue is: AppDomain.CurrentDomain.ProcessExit += delegate { Console.WriteLine("Process exit"); };
Thread.Sleep(int.MaxValue); If you send |
Yes, it is the same root cause. The |
|
attn @SamMonoRT |
So the way this seems to work on coreclr is that there's a background thread that waits for commands, among them "process shutdown". the sigterm signal handler writes a "process shutdown" command. That wakes up the thread and it begins the shutdown of the process. We end up in We already have I guess the missing bit is to install a signal handler for SIGTERM, have it set some flag and wake the finalizer thread (using |
We did have a SIGTERM handler in mono/mono which was used for crash reporting functionality, it was removed from dotnet/runtime in 0c90347 |
I have experimented with installing a
The handler at address
where -4 == The signal handler in that module will process the signal and if there is an existing handler will invoke it as well. Unless it is one of the non-cancelable termination signals:
Where
Therefore, our signal handler is skipped and we don't get to do the |
Web applications cancel As a test app you can use something like: AppDomain.CurrentDomain.ProcessExit += delegate { Console.WriteLine("Process exit"); };
Thread.Sleep(int.MaxValue); |
Console AppI am seeing that "Process Exit" message with my patch for the simple console app. I don't see it using the unpatched code. However, while I see the message the program itself keeps on sleeping:
Web AppI see the "Process Exit" with both patched and unpatched .NET 7. This is the app run on both:
|
I need to revisit the tests as I was inconsistent in sending the signal to the correct process. I'll redo in my morning (UTC+10). |
Revisited: ConsoleWithout PatchKilling
So an orphaned process but returned to prompt. With Patch
Process(es) don't terminate.
WebWithout Patch
With Patch
Process(es) don't terminate.
Further AnalysisIf I change the console app to do:
Then after the If I change it to:
Then the writing of the The PatchHere's the patch. The area of concern is where it is invoked in the finalizer thread and whether any further action is required (break? set finished to true? something else?).
|
CoreCLR doesn't wait for |
That does the trick for mono as well. Both the Before SIGTERM
After SIGTERM
I instrumented the mono process to show the variable being set and the shutdown taking effect:
The patch to
Is a status of 0 what we want? |
On This behavior is captured by the |
Updated the patch so that we:
Now when I send a
Strangely, with the
|
A user should still be able to set the ExitCode, as tested by this case:
|
Mono has a |
The
An
I changed the
And ran things again and this time looked at all the threads:
The
So I think it's doing what it should. Should I generate a PR? |
The VBCSCompiler sticking around is expected, it's the Roslyn compiler server: https://github.com/dotnet/roslyn/blob/main/docs/compilers/Compiler%20Server.md. This is probably because you use |
@nealef definitely! |
"Mono does not emit ProcessExit event on SIGTERM" * src/mono/mono/mini/mini-posix.c - Add signal handler for SIGTERM - SIGTERM handler will set a global variable that may be monitored by the GC finalizer thread * src/mono/mono/metadata/gc.c - Monitor for sigterm and kick off the shutdown process when encountered by calling mono_runtime_try_shutdown(). - Exit with either the user set exitcode (System.Environment.ExitCode) or SIGTERM + 128.
…on SIGTERM" * src/mono/mono/mini/mini-posix.c - Add signal handler for SIGTERM * src/mono/mono/mini/mini-windows.c - Add signal handler for SIGTERM * src/mono/mono/mini/mini-runtime.c - Add mono_sigterm_signal_handler to process SIGTERM that will set a global variable to be monitored by the GC finalizer thread * src/mono/mono/mini/mini-runtime.h - Define prototype for mono_sigterm_signal_handler() * src/mono/mono/metadata/gc.c - Monitor for sigterm and kick off the shutdown process when encountered by calling mono_runtime_try_shutdown(). - Exit with either the user set exitcode (System.Environment.ExitCode) or SIGTERM + 128.
@nealef - your PR addresses the issue, should we close this one ? |
It doesn't address it for Windows. I am attempting to mimic the behaviour of coreCLR but haven't had enough free cycles to get it done.
|
Moving this to the 9.0.0 milestone. If the fix is trivial and non-risky we will consider backport to 8.0 build prior to release. |
…on SIGTERM" * src/mono/mono/mini/mini-posix.c - Add signal handler for SIGTERM * src/mono/mono/mini/mini-windows.c - Add signal handler for SIGTERM - Use the correct signal for handler * src/mono/mono/mini/mini-runtime.c - Add mono_sigterm_signal_handler to process SIGTERM that will set a global variable to be monitored by the GC finalizer thread - Set a default exit code before setting the term_signaled variable that gets checked in gc * src/mono/mono/mini/mini-runtime.h - Define prototype for mono_sigterm_signal_handler() * src/mono/mono/metadata/gc.c - Monitor for sigterm and kick off the shutdown process when encountered by calling mono_runtime_try_shutdown(). - Exit with either the user set exitcode (System.Environment.ExitCode) or SIGTERM + 128. - Simplify use of exit code now that a default is being set - Rename term_signaled to match mono style - Remove volatile attribute - Move testing of shutdown until after the sem wait * src/libraries/System.Runtime/tests/System/ExitCodeTests.Unix.cs - Re-enable ExitCodeTests for mono * src/mono/mono/mini/exceptions-amd64.c src/mono/mono/mini/exceptions-x86.c - Add control event handling for windows
…TERM" (#100056) * src/mono/mono/mini/mini-posix.c - Add signal handler for SIGTERM * src/mono/mono/mini/mini-windows.c - Add signal handler for SIGTERM - Use the correct signal for handler * src/mono/mono/mini/mini-runtime.c - Add mono_sigterm_signal_handler to process SIGTERM that will set a global variable to be monitored by the GC finalizer thread - Set a default exit code before setting the term_signaled variable that gets checked in gc * src/mono/mono/mini/mini-runtime.h - Define prototype for mono_sigterm_signal_handler() * src/mono/mono/metadata/gc.c - Monitor for sigterm and kick off the shutdown process when encountered by calling mono_runtime_try_shutdown(). - Exit with either the user set exitcode (System.Environment.ExitCode) or SIGTERM + 128. - Simplify use of exit code now that a default is being set - Rename term_signaled to match mono style - Remove volatile attribute - Move testing of shutdown until after the sem wait * src/libraries/System.Runtime/tests/System/ExitCodeTests.Unix.cs - Re-enable ExitCodeTests for mono * src/mono/mono/mini/exceptions-amd64.c src/mono/mono/mini/exceptions-x86.c - Add control event handling for windows Co-authored-by: Jo Shields <directhex@apebox.org>
To reproduce:
Find the pid of the
dotnet run
process, and send it SIGTERM from another terminal:With CoreCLR, the application shuts down, and the exit code is
0
:With Mono, the application keeps running, the
dotnet run
process gets terminated:The
web
process is still running, you can send it SIGTERM seperately and it terminates nicely.This difference in behavior is causing one of the source-build smoke tests to fail: dotnet/source-build#3174.
cc @akoeplinger
The text was updated successfully, but these errors were encountered: