Skip to content
This repository was archived by the owner on Apr 20, 2023. It is now read-only.

Add support in dotnet run for pid files#1393

Closed
victorhurdugaci wants to merge 1 commit intorel/1.0.0from
victorhu/pid-file
Closed

Add support in dotnet run for pid files#1393
victorhurdugaci wants to merge 1 commit intorel/1.0.0from
victorhu/pid-file

Conversation

@victorhurdugaci
Copy link
Copy Markdown

Add support for dropping a pid file. The watcher needs it because otherwise it cannot kill the process started by dotnet run

cc @piotrpMSFT @anurse

syntax.DefineOption("f|framework", ref runCmd.Framework, "Compile a specific framework");
syntax.DefineOption("c|configuration", ref runCmd.Configuration, "Configuration under which to build");
syntax.DefineOption("p|project", ref runCmd.Project, "The path to the project to run (defaults to the current directory). Can be a path to a project.json or a project directory");
syntax.DefineOption("pf|pid-file", ref runCmd.PidFile, "A file that contains the process id of the executed application");

This comment was marked as spam.

This comment was marked as spam.

@ThatRendle
Copy link
Copy Markdown

I've been wishing CoreFx on POSIX systems had a way to listen for SIG*.

@Petermarcu
Copy link
Copy Markdown
Member

@stephentoub, @Priya91 , Thoughts? We've had this item in the backlog for a while. https://github.com/dotnet/corefx/issues/3188

@stephentoub
Copy link
Copy Markdown
Member

I'm not following the thread, but if the concern is around lack of graceful shutdown in response to SIGTERM, there's https://github.com/dotnet/coreclr/issues/2688

@victorhurdugaci
Copy link
Copy Markdown
Author

@stephentoub we need this feature for dotnet watch: watch calls dotnet run which starts the application that was built. The problem is that the watcher has no idea about the second process and when it kills dotnet run, the second process is orphaned. With the pid file, run will let the watcher know what's the actual process being watched.

@stephentoub
Copy link
Copy Markdown
Member

@victorhurdugaci, thanks, but I'm not understanding the connection to the comments about graceful shutdown and signal handling. Can you elaborate? What feature of corefx/coreclr is desired here to address this situation?

@victorhurdugaci
Copy link
Copy Markdown
Author

@stephentoub we just need a cross platform mechanism to signal and gracefully shutdown a process. That way, dotnet run could stop the child process and the watcher wouldn't be aware of it. Today, Process.Kill doesn't give the chance to do any cleanup.

@stephentoub
Copy link
Copy Markdown
Member

Ah, I understand now.

Well, using AssemblyLoadContext.Unloading (https://github.com/dotnet/corefx/blob/master/src/System.Runtime.Loader/ref/System.Runtime.Loader.cs#L37) you can register to be notified when your process is gracefully exiting (e.g. because Environment.Exit is called), and with https://github.com/dotnet/coreclr/issues/2688 (cc: @kouvel) this should be triggered in response to a SIGTERM signal.

It sounds then like then you're simply wanting the ability to send a SIGTERM on Unix, and you can do so with a P/Invoke to kill(pid, 15) conditionally when RuntimeInformation.IsOSPlatform(OSPlatform.Windows) is false. I agree we should at some point wrap kill to allow for sending arbitrary signals without having to write your own P/Invoke (that's part of what https://github.com/dotnet/corefx/issues/3188 is about), but it's a very easy P/Invoke to write, and if we did wrap it in a new corefx library, it would be platform-specific, since signals are platform-specific. I'm not even sure what a cross-platform "please shutdown gracefully that process over there" API would look like... what would you do on Windows to request another process gracefully shut down? In particular, how would you do it for a process that's not running a message loop to receive WM_QUIT or WM_CLOSE messages?

@stephentoub
Copy link
Copy Markdown
Member

Or, are you asking for something else, like the ability to manually send (the equivalent of) ctrl-C to another process on both Windows and Unix?

@davidfowl
Copy link
Copy Markdown
Member

Kill and entire process tree

@victorhurdugaci
Copy link
Copy Markdown
Author

@stephentoub @davidfowl yup, ultimately the goal is to kill an entire process tree

@stephentoub
Copy link
Copy Markdown
Member

Since you're already in the world of launching this or that utility, have you considered just launching the various utilities to kill a whole process tree, e.g. pkill -TERM -P pid on Unix and taskkill /t /pid pid on Windows?

@victorhurdugaci
Copy link
Copy Markdown
Author

@stephentoub that's an interesting idea which might work. Let me give it a try and maybe we don't need this PR after all

@victorhurdugaci
Copy link
Copy Markdown
Author

@stephentoub that seems to work. I'll close this PR for now

@muratg
Copy link
Copy Markdown

muratg commented Feb 23, 2016

@stephentoub Apparently taskkill.exe doesn't exist on Nano... any suggestions there?

@stephentoub
Copy link
Copy Markdown
Member

Apparently taskkill.exe doesn't exist on Nano... any suggestions there?

I don't know what utilities are available there.

I don't have a great suggestion with what's available... even in the full framework, there's no Process.Kill(bool recursive) or anything like that, and folks have resorted either to using taskkill.exe or to using WMI. If Process exposed a ParentId, then that could be used to implement this as well, as one could then determine from the list of processes all of the processes which should be killed individually... unfortunately again there's no such property in the framework (FWIW, my initial guess at how such a potential future recursive Kill would be implemented is basically that way). cc: @pallavit in case she has any other ideas.

There are other workarounds one could imagine. For example, the parent process could pass its ID (Process.GetCurrentProcess().Id) to the child process, which could then poll to see if the parent is still around, and if it's not, Environment.Exit(). Not ideal, but potentially workable.

@TheRealPiotrP TheRealPiotrP deleted the victorhu/pid-file branch July 26, 2016 23:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants