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

PR_SET_PDEATHSIG support on linux #96470

Closed
elijahr2411 opened this issue Jan 4, 2024 · 6 comments
Closed

PR_SET_PDEATHSIG support on linux #96470

elijahr2411 opened this issue Jan 4, 2024 · 6 comments

Comments

@elijahr2411
Copy link

When using System.Diagnostics.Process on Linux, there should be a way to specify that the PR_SET_PDEATHSIG flag should be set on the child process using the prctl() syscall before forking. This tells the kernel to send a signal (usually SIGINT or SIGTERM) to the child when the parent process dies. This is useful for scenarios where the user wants the child process to be killed in cases where the parent exits uncleanly (such as an exception), as otherwise the child process will remain running after the parent exits in such ways.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 4, 2024
@ghost
Copy link

ghost commented Jan 4, 2024

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Issue Details

When using System.Diagnostics.Process on Linux, there should be a way to specify that the PR_SET_PDEATHSIG flag should be set on the child process using the prctl() syscall before forking. This tells the kernel to send a signal (usually SIGINT or SIGTERM) to the child when the parent process dies. This is useful for scenarios where the user wants the child process to be killed in cases where the parent exits uncleanly (such as an exception), as otherwise the child process will remain running after the parent exits in such ways.

Author: elijahr2411
Assignees: -
Labels:

area-System.Diagnostics.Process, untriaged

Milestone: -

@jozkee
Copy link
Member

jozkee commented Mar 13, 2024

Would you like to draft an API proposal? Also, what would such API do for Windows?

@jozkee jozkee added this to the Future milestone Mar 13, 2024
@jozkee jozkee removed the untriaged New issue has not been triaged by the area owner label Mar 13, 2024
@elijahr2411
Copy link
Author

elijahr2411 commented Mar 15, 2024

Would you like to draft an API proposal?

Not really my expertise but I could definitely give it a shot!

Also, what would such API do for Windows?

It would most likely be a platform-specific feature as windows does not have an equivalent to PR_SET_PDEATHSIG as far as my limited knowledge of that platform goes. I suppose something with job objects is possible, however

@dmitry-azaraev
Copy link

PR_SET_PDEATHSIG sends signal when parent thread dies. This means what your app should use dedicated separate thread (or even dedicated thread pool if you spawn lot of processes concurrently) - and such decision in my opinion should not be taken by core library, and should be handled by application explicitly (as this threads probably can be used for other purposes - excess useless threads are evil).

Killing childs on Windows is doable with Job Objects, and this is trivial. But again, race-free implementation should allocate & assign them at process creation. But Job Objects so big/flexible, what it will be sad design to use it only for JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE.

What with other platforms? I'm don't know.

This leads what while there is technically possible unify this functionality under simple flag, but, IMO, it is much better to not unify and expose OS-specific interfaces instead. However, from my understanding, .NET libraries follow different philosophy.

However, this is popular functionality request, probably my concerns is not too important.

@adamsitnik
Copy link
Member

Let's continue the discussion in #101985

@adamsitnik adamsitnik closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
@dmitry-azaraev
Copy link

@adamsitnik no offence, but i don't interested in this. I already provided pointers, but i will not continue in discussion: dotnet sd process is badass and it can't be improved at all - it do things which never needed. It never will fill all my needs, so it is really better to pinvoke to do os specific job. SD.Process was bad right from .NET 1.0 - and keep fucking this stupid interface never will made things better in my taste if we extend it to some degree.

Just let us work with native calls, we did not care about this stupid "portable" things, which eventually throw instead of be non-compilable.

So, thanks for yours mention, but in reality anyone who want this - done for self. Who did not want - is exist some good third-parties. No good third-parties actually exist, just like .net implementation doing all things wrong. Inheriting handles nowadays must be specified explicitly.

Again, no offense,bit is good what this method work across wide platforms, but gling deeper - isvery os specific. You never get unified interface to cover all oses,because they are different. And ONLY ONE correct way do this - did not allow compile code for missing methods. Bye-bye, but .net going different in core libs, and so i feel no any idea how passing handles or mapping fds might work in unified interface. They will not. Additionally there exist lot of other great to see options... But main one is to stop proxying strio at all. Let us pass FDs and did not touch them.

TLDR: SD.Process fundamentally broken. I never will use it, no matter how it will be improved. It just shit which never should exist (bad interface design from 2000). So... you got my point. I presonally see zero space to improvement, because base impl is stupid,not extensible and etc.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants