tugberkugurlu
commented
8 years ago does this need https://github.com/dotnet/coreclr/issues/2688 to be finished first?
Closed Petermarcu closed 4 years ago
tugberkugurlu
commented
8 years ago does this need https://github.com/dotnet/coreclr/issues/2688 to be finished first?
stephentoub
commented
8 years ago does this need dotnet/coreclr#2688 to be finished first?
No, it shouldn't.
karelz
commented
8 years ago We need API proposal
chadwackerman
commented
7 years ago Does "We need API proposal" actually count as managing this issue? (Are you really managing one of the largest and most out of control projects at Microsoft using GitHub tags @karelz? Folksonomy doesn't even scale to the average knitting blog.)
Isn't this just a couple of event handlers? Where do you want to put the non-obvious ones?
AssemblyLoadContext.Default.Unloading += MethodInvokedOnSigTerm; // an obvious one@chadwackerman I am not sure what exactly is your question here. Yes, we are using GitHub tags for categorizing issues -- here's more details about it. The comment "We need API proposal" is meant to suggest the next step (see details here).
You can learn more about formal API proposals.
All docs should be linked from project root.
Isn't this just a couple of event handlers?
Possibly, that's exactly what we expect from API proposal - someone to think deeply about it and propose API with story about it (why, how, what, rejected alternatives, etc.).
Please let's stay technical (see contributor code of conduct).
chadwackerman
commented
7 years ago Is there a tag link I can click on that shows items waiting for an API proposal? I don't see one here. And are you seriously (and silently!) asking the community to map signal.h to an enum and delegates?
Microsoft already owns shipping code that does this. It's called Mono.
You can click the linked issues around Unix signals in this GitHub project and it takes you in a circle, with nobody doing anything. You can actually use the PowerBI GitHub adaptor to show that Microsoft employees are (I hope unconsciously!) blocking on one another. It's almost as bad as those Microsoft Support sites where three Microsoft subcontractors upvote each other's wrong answers. (But hey, the time to close metrics sure look great!)
With all due respect I think the process here is failing you. Somebody needs to take ownership of an issue, not just tag and link to other issues.
Meanwhile...
I'm running a service on Azure Batch and it actually returns "success" from a cloud batch job if the exit code is nonzero. Why? Because Robocopy sometimes returns a 1 to indicate success.
I'm trying to use the OneDriveSDK and the devs there swear it works cross platform and close issues. But it has dependencies on WinForms and STAThread so it doesn't even work in IIS let alone the command line on Linux.
Either you're gonna support Linux or not. Perhaps starting with signal.h and honoring 0=success as a return code from main is a good start? .NET already whiffed on arg[0] being the name of the app, but maybe we can slowly turn this boat around. But I don't believe the solution is a few more captains and a lot more tagging.
karelz
commented
7 years ago Is there a tag link I can click on that shows items waiting for an API proposal?
All api-needs-work items are API additions which are not api-approved and not api-ready-for-review (see API review process)
... are you seriously (and silently!) asking the community to map signal.h to an enum and delegates?
We are seriously (and loudly!) asking anyone who will grab this issue to provide a proposal -- it may be MS employee or it may be community member, we are treating people equally in this matter. If it is a simple mapping of signal.h, then be it. Maybe it is more complicated. (I don't know, because I didn't look deeper into the issue - that's what the person grabbing it is supposed to do)
Microsoft already owns shipping code that does this. It's called Mono.
Leveraging Mono code/design is very reasonable approach. We also do it where it helps. Whoever gets to work on this one first is free to consider it.
You can click the linked issues around Unix signals in this GitHub project and it takes you in a circle, with nobody doing anything.
I am not sure which link-circles you refer to, but even the Milestone set to Future on this issue is supposed to set the expectation that CoreFX team does not consider it as something must-have for next version (2.0), so the "nobody doing anything" is expected at least from MS employees. If community members believe we are misjudging the priority, we encourage them to either contribute or help us understand the impact of the issue better (in a constructive polite way please).
You can actually use the PowerBI GitHub adaptor to show that Microsoft employees are (I hope unconsciously!) blocking on one another.
Never tried PowerBI adaptor. I am not sure how it would help us. This particular issue is blocked only by "resources" (i.e. someone doing the job). In general, I think I have high-level insight into most of the repo issues and areas and I am pretty sure there is very nearly zero issues where MS employees are blocked on one another in CoreFX repo.
@chadwackerman maybe it will be a surprise to you, but we (CoreFX teams) are not bunch of lazy or ignorant engineers. We are trying to do the right things the right way. I can't force you to believe that, but I will at least ask you to be constructive and technical in your criticism -- we are open to arguments and discussions. Throwing in general statements and unfounded accusations is not going to help anyone (and I plan to stop responding to them, potentially start deleting them).
On the other hand, if someone truly wants to understand the motivation for our process or repo organization, I'll be very happy to explain (and eventually adjust docs to capture it). We are also open to feedback on the process/docs, especially if it is feedback supported by larger part of our community.
Somebody needs to take ownership of an issue, not just tag and link to other issues.
Until the issue is assigned to someone it does not have an "owner". The area experts help watch over all issues in their areas and help identify bugs which might have been incorrectly prioritized or where new information arises from the discussion. This particular issue has 7 votes so far and does not have any evidence that the missing API is blocking major work on .NET Core. If you believe this is wrong assessment, you have 2 options: Convince us (with evidence and motivation) that it is higher importance, or contribute.
I'm running a service on Azure Batch and it actually returns "success" from a cloud batch job if the exit code is nonzero. Why? Because Robocopy sometimes returns a 1 to indicate success.
I don't see how it is related to this issue.
I'm trying to use the OneDriveSDK and the devs there swear it works cross platform and close issues. But it has dependencies on WinForms and STAThread so it doesn't even work in IIS let alone the command line on Linux.
Please work with their team. CoreFX is not the right place to raise that issue.
Either you're gonna support Linux or not. Perhaps starting with signal.h and honoring 0=success as a return code from main is a good start?
I fail to see how it is relevant to this issue. If you think there is a bug in .NET Core apps returning non-0 value by default, please file it as separate issues and let it be discussed on its own. Let's not entangle issues together please, unless they are truly relevant e.g. in overall end-to-end impact.
iam3yal
commented
7 years ago @chadwackerman If you want to give them feedback about their process or file a bug you can always open a new issue about it but please don't derail the issue at hand.
p.s. Just a piece of friendly advice, you can lose the attitude, no one awe you anything.
chadwackerman
commented
7 years ago @eyalsk This isn't a hobbyist's blog engine, this is a core technology from a company worth nearly half a trillion dollars. They're charging me by the minute to run my Linux dotnet apps and they very much do owe me something.
Among the other candy-colored rectangles I see "up for grabs" and "backlog" but "only 7 votes" and "not expected for v2.0." Meanwhile there's many related bugs and lots of StackOverflow traffic when you Google the issue, which is how I landed here. Apparently I need to build a political action committee or hire lobbyists since we're voting on technology now?
We have a senior Microsoft manager who chimes in with "We need API proposal" - end of sentence, no punctuation. The unintentional effect of which is Cookie Licking as it appears the true answer is "Yes, we are expecting the community to solve signal.h despite everyone from Satya down to Scott Hanselman saying we're all aboard the Linux train."
I don't see how it is related to this issue.
There's a LOT of really amateurish stuff going on at the command line with the various Microsoft teams and it doesn't surprise me that nobody is jumping in to tackle this issue. "return 0" means success, stop hooking my app's signals with your wonky libraries without letting me override, and maybe cut down on the verbosity of the compilers because what it spits out reads like second year college projects where you can feel the youthful excitement seeping out of the passively-voiced text:
"Project will be compiled because inputs were modified... Project will be compiled because expected outputs are missing... Time elapsed 00:00:02.2927068"
I realize .NET format strings are confusing but sheesh, this ain't the Carnegie Mellon University Pascal compiler lab circa 1990 fellas.
iam3yal
commented
7 years ago @chadwackerman
This isn't a hobbyist's blog engine, this is a core technology from a company worth nearly half a trillion dollars. They're charging me by the minute to run my Linux dotnet apps and they very much do owe me something.
Can you create a new issue about your problem?
Among the other candy-colored rectangles I see "up for grabs" and "backlog" but "only 7 votes" and "not expected for v2.0." Meanwhile there's many related bugs and lots of StackOverflow traffic when you Google the issue, which is how I landed here. Apparently I need to build a political action committee or hire lobbyists since we're voting on technology now?
Can you create a new issue about it? i.e. giving them feedback about their process.
There's a LOT of really amateurish stuff going on at the command line
Can you create a new issue about it?
with the various Microsoft teams and it doesn't surprise me that nobody is jumping in to tackle this issue. "return 0" means success,
Same as above.
stop hooking my app's signals with your wonky libraries without letting me override,
Same as above
and maybe cut down on the verbosity of the compilers
Same as above.
I'm sure they will want to know about the problems you're having.
chadwackerman
commented
7 years ago @eyalsk I'm sure you're the first guy to grab the Scrabble box for rules clarifications. Unfortunately adding another five tiny issues to the massive backlog here is unlikely to move the needle. Which if you'll take a few minutes away from your busy schedule spent starring repositories for no reason, you may realize indicates a bigger problem with the process here. Thus my raising the issue the way I did.
We're dealing with serious technical issues by clicking TiVo thumbs up/down buttons and there's a grown man who works at Microsoft getting stock awards for taking it seriously and parroting four word responses back to us. Moreover there is no sane path to discuss strategic issues which pushes all this toward the inevitable marketing and social circlejerk. Seriously--write a bot to crawl these issues links and they all point to each other, all stuck on each other. We're watching Vista unfold in real time here.
Sure, I can email a few buddies and get this bumped from 7 to 20 upvotes. Meanwhile there's critical stuff with thousands of votes ignored for years so what's the point? I'm not sure what personality type is motivated to dive into this particular flavor of open source mess, but just as soon as @karelz finds a few minutes to tinker with the GitHub to Microsoft BI adaptor I'm sure he'll post a few numbers in support of his favorite bugs.
And I were going to donate my time, I can think of a few thousand organizations more needy than the corporation that has the most cash on hand of any entity on Earth. But that's just me.
robertmclaws
commented
7 years ago @karelz Can I just say that, reading through this thread, there is legitimate criticism here that can easily be addressed. Microsoft employees might consider not being so defensive... you guys created a HUGE mess for the ecosystem. It's your job to sort it out. It's not our fault the team decided to boil the ocean... do you think the Vista team didn't have to deal with disgruntled people?
There's a real simple solution. Just say "thanks for your feedback, we'll take it into consideration" and move on. You don't HAVE to respond to every single piece of criticism, but you definitely should not be deleting it, either.
iam3yal
commented
7 years ago @chadwackerman
I'm sure you're the first guy to grab the Scrabble box for rules clarifications. Unfortunately adding another five tiny issues to the massive backlog here is unlikely to move the needle. Which if you'll take a few minutes away from your busy schedule spent starring repositories for no reason, you may realize indicates a bigger problem with the process here. Thus my raising the issue the way I did.
People star and/or watch repos because they are/were using them or because they know they will need to use them at some point or just because they want to follow the development so there are good reasons for it.
People that value their time, lose their attitude and don't waste their time on making stupid assumptions, bragging or trolling and finally expect people to take them seriously.
If you don't trust Microsoft and/or their process either give them feedback about it, writing something is always better than nothing or shut it!
I actually need this feature because I'm working on a console application and I want to move it to .NET Core and I want to intercept SIGINT when the user exit the application.
We're dealing with serious technical issues by clicking TiVo thumbs up/down buttons and there's a grown man who works at Microsoft getting stock awards for taking it seriously and parroting four word responses back to us. Moreover there is no sane path to discuss strategic issues which pushes all this toward the inevitable marketing and social circlejerk. Seriously--write a bot to crawl these issues links and they all point to each other, all stuck on each other. We're watching Vista unfold in real time here.
Sure, I can email a few buddies and get this bumped from 7 to 20 upvotes. Meanwhile there's critical stuff with thousands of votes ignored for years so what's the point? I'm not sure what personality type is motivated to dive into this particular flavor of open source mess, but just as soon as @karelz finds a few minutes to tinker with the GitHub to Microsoft BI adaptor I'm sure he'll post a few numbers in support of his favorite bugs.
thumbs up/down are just reaction to a specific issue, I doubt that Microsoft prioritize their work on the amount of thumbs up, it might be an indication that people need this feature but that's all, it's just another form of feedback.
And I were going to donate my time, I can think of a few thousand organizations more needy than the corporation that has the most cash on hand of any entity on Earth. But that's just me.
Have fun.
stephentoub
commented
7 years ago I want to intercept SIGINT when the user exit the application.
@eyalsk, can you use Console.CancelKeyPress? It raises an event for SIGINT and let's you handle it.
iam3yal
commented
7 years ago @stephentoub I haven't tried it yet, I'm still digging what it would take for me to port the application.
Does everything related to Console.* works on .NET Core?
stephentoub
commented
7 years ago On Windows, pretty much all of console works as it does on desktop. On Unix, most of the support works as you'd expect, but some of the more esoteric functionality (e.g. screen buffers) is limited.
tmds
commented
7 years ago Thinking about some use-cases I have for signals, I want to propose some API calls. @Petermarcu and others, please provide feedback if these calls would cover your use-cases.
These are members of a static Signal class.
A common use-case is to send a signal to a process to kill or term it:
Signal.Send(Process process, SignalValue signal)This call would probably be used in a while loop with Process.WaitForExit.
Another interesting use-case is sending a signal to a specific thread to unblock it. Perhaps some native method was invoked on this thread which can be stopped by sending it a specific signal.
Signal.Send(Thread thread, SignalValue signal)This call would probably be used in a while loop with Thread.Join.
There are several signals which can be used for user-defined signals (sigusr, real time). As signals are process wide, it may be interesting to have a way of acquiring a signal for 'exclusive' use.
SignalValue Signal.AquireSignal(); // may return a value indicating there are no more signals available
Signal.ReleaseSignal(SignalValue value);One interesting case is a handler which has an empty body and is used to unblock system calls. This handler may be used by different consumers since it is a no-op.
SignalValue Signal.EmptyActionSignal { get; }We need a way to block and unblock signals.
Signal.Block(params SignalValue[]);
Signal.Unblock(params SignalValue[])These operations are at the thread level. So perhaps they should be allowed only on specific types of threads (e.g. not on the thread pool).
-edited- Creating a new thread via pthread_create will copy the signal mask (so will fork). It may make sense to control the signal mask at process startup and at thread creation. For example, at process startup all user signals and ignored signals could be blocked and at thread start all signals could be blocked. This would allow the user to fully control which threads handle what signals. This would be a breaking change from what is happening in the existing implementation.
Adding a handler:
// return value can be used to remove the handler
IDisposable Signal.AddHandler(SignalValue value, Action action);This would keep a list of actions per signal value. When the first action is added, the signalaction would change to execute the handlers. When the last action is removed, the signalaction would revert to the default.
As process directed signals may be handled by any unblocked thread, it is important the user realizes a call to AddHandler may cause the action to be executed if another thread has Unblocked it. This is where the AquireSignal comes into play which allows a user to reserve a signal that should not be used by others.
Since not all functions can be called from a signal handler, it may make sense to defer the action to the threadpool and block the signal handler until it completes.
SignalValue in the above is an enum with names of signals. It has to be taken into account that the numeric values are not fixed.
Petermarcu
commented
7 years ago @stephentoub , can you take a look?
stephentoub
commented
7 years ago Thanks for your thoughts on this, @tmds!
It's great to discuss all of the various use cases folks may have, so keep that coming. At the same time, this is a fairly large area, with many intricacies that will impact things, e.g. we can't really reasonably run managed code in a signal handler, as aspects of the runtime that could get involved (e.g. JIT, GC, etc.) almost certainly use functionality that's not signal-safe, nevermind the code in the handler itself, libraries they use, etc. And we need to be concerned about existing functionality that's already using/handling signals in the runtime and other CoreFx libraries, e.g. SIGINT and Console.CancelKeyPress, SIGTERM and polite shutdown, debugger interaction, etc.
So, I suggest we split this into multiple pieces:
Getting the results for (1) would be awesome, if someone's interested in tackling that, and I expect that will help with the rest of this. I then suggest we leave (4) aside for a bit and tackle (2) and (3) together: I expect (3) should be fairly straightforward to design, and we could then make things much more concrete by getting that as a manageable chunk designed, reviewed, implemented, and merged. Then with that in place as a basis, along with the intel from (1), I expect it would be very helpful for someone to prototype (4), to help understand what works and what doesn't, what issues we'll hit, etc., and use that along with (1) to drive a design for (4), which can then be reviewed, designed, implemented, and merged.
Does that seem reasonable? @tmds, you seem fairly passionate about this, and it's great to see all of the thoughts you've already outlined. Would you want to tackle (1) as a start?
iam3yal
commented
7 years ago @tmds
Isn't it better to call SignalValue something like POSIXSignal or UnixSignal it seems a bit more descriptive.
@stephentoub
Understand what other platforms / languages / libraries do in this space. As input to all of this, it would be helpful for someone to review and summarize what other platforms do in this regard, e.g. what do APIs for this look like in Java, node.js, Python, Go, etc., and how are they typically consumed, e.g. common use cases in code to help highlight the kinds of things that would need to be supported. What problems have they hit? What do devs love? What gaps do they still have that folks complain about? Also, it looks like Mono already has some related support, e.g. its UnixSignal. It'd be great to understand what Mono provides in this area, whether it's met folks' needs or whether there are big gaps, and use that to drive this design. Could we just use what Mono already has?
I can speak for node.js (Node) for now.
Sending singnals on Node is as simple as doing something like this:
const process = require("process");
process.on("exit", (code) => {
console.log("EXIT");
}); Works both on Windows and Linux.
process.on("SIGKILL", (code) => {
console.log("SIGKILL");
}); This will throw an error on all platforms but can be sent through process.kill(process.pid, "SIGKILL");
This is what the documentation for Node 7.4.0 states:
Windows does not support sending signals, but Node.js offers some emulation with process.kill(), and ChildProcess.kill(). Sending signal 0 can be used to test for the existence of a process. Sending SIGINT, SIGTERM, and SIGKILL cause the unconditional termination of the target process.
As far as I can tell all signals are supported but some features may or may not be supported or have different behaviour on Windows for example:
SIGWINCH: is delivered when the console has been resized. On Windows, this will only happen on write to the console when the cursor is being moved, or when a readable tty is used in raw mode.
Personally I don't think it's correct for a signal to send itself because Signals are related to processes so I don't think it's correct to say Signal.Send(...).
You can check the documentation for more information about it here and here.
Based on this information I'm thinking that the API should be something like this:
public enum UnixSignal
{
// ...
SIGKILL = 9
// ...
}
static class ProcessExtensions
{
public static int SendSignal(this Process process, UnixSignal signal)
{
// Returns an error code
return default(int);
}
}Usage:
Process process = new Process();
process.SendSignal(UnixSignal.SIGKILL);Just a thought but we can also have a specific enum for each platform like so:
public enum WinSignal
{
// ...
SIGKILL = 9
// ...
}This will basically be mapped to UnixSignal so you can use either but people that say target only Windows and want to use signals for whatever reason and are interested in signals that are guaranteed to work on Windows may choose to use WinSignals over UnixSignal but this is just an idea.
So there might be an overload method that looks like this:
public static int SendSignal(this Process process, WinSignal signal)
{
if (!Enum.IsDefined(typeof(UnixSignal), (int)signal)) throw new ArgumentException();
return process.SendSignal((UnixSignal)signal);
}
jnm2
commented
7 years ago Just a thought but we can also have a specific enum for each platform
If it's possible to create a non-leaky abstraction, I'd prefer to work with an abstraction and write platform-agnostic code.
I'd probably hope for something similar to:
public enum Signal
{
Kill,
UnixTerm
}@jnm2 Sure but seeing how it's implemented in other platforms where the names are kept then I thought it's a good idea to follow it to some extent.
As far as I can tell all of them use names that match the Unix names, I don't mind changing the names to pascal casing as opposed to upper casing and give them more readable names but I still think that the name of the enum should be UnixSignal. :)
UnixSignal.Kill, UnixSignal.Term makes sense to me even though it might be an abstraction and supported by all platforms.
p.s. I'm not aware of anything like this in the Java world or more precisely you can't send signals directly with Java, at least without some 3rd-party library that provides it.
chadwackerman
commented
7 years ago This Java product seems to have influenced Mono.
http://www.bmsi.com/java/posix/index.html
A bigger issue (and source of future breaking changes) is that the .NET Core team is hopping along with their process and console stuff with little consideration of the cross-platform requirements. You should start bottom up with signals not slap a GitHub kiss-of-death "backlog" button on it. It's a core concept everyone needs to understand and affects your designs today.
stephentoub
commented
7 years ago with little consideration of the cross-platform requirement
Cross platform is a key aspect of everything new we add. I'm not sure what you're referring to around Process and Console; the only APIs there are ones that already exist in .NET, and such compat is another key aspect. I believe we are starting fresh for signals, which is why I asked for background on use cases, on other platform designs, etc. Let's keep this thread focused on the design and refrain from higher level commentary on processes by which this repo is managed: you've made your opinions clear, they've been noted, and they'll be considered for the future; if you have other suggestions on processes related to the repo, please open a separate issue. Thanks for the link.
chadwackerman
commented
7 years ago There's already legacy gunk in Console app startup that's going to break future Signals work. The .NET team was alerted to this by Miguel ten years ago. As for cross-platform: you can't tell me with a straight face that Microsoft seriously considered SIGTERM vs SIGINT while planning the development arc of .NET Core's console app functionality. That's why this bug exists. And more importantly why it was then buried and scheduled for consideration sometime in 2025.
I'll promise to be nicer if Microsoft promises to be a little less full of crap. And I'm still not sure why you're dragging the "community" into this when the first step should be to talk to the Mono guys down the hall. But you got the obvious wrongheaded design and three Google links instead of typing the search in yourself, so engagement metrics are up. Carry on.
tmds
commented
7 years ago @stephentoub tacking (1)
Sending signals to a process:
func (p *Process) Signal(sig Signal) errorprocess.kill(pid[, signal])os.kill(pid, sig)public static int kill (int pid, Signum sig)Sending signals to a process group:
os.killpg(pgid, sig)(note for this to work a process must call setpgid between fork and execve, a process started with the Process class doesn't provide a flag to enable this) Sending signals to a thread:
signal.pthread_kill(thread_id, signalnum)note currently there is no API to retrieve the native thread id: pthread_t of a thread created via new Thread.
Handling signals: go handling is described here: https://golang.org/pkg/os/signal/ nodejs handling is described here: https://nodejs.org/api/process.html#process_signal_events For both go and nodejs, it is specified what signals can be 'user handled'. If no user handler is installed a default action occurs. go allows also to explicitly set the action to ignore. The signal handling is considered a process level activity (e.g. the runtime ensures there is a thread that is unblocking the proper signals). add a handler:
go: func Notify(c chan<- os.Signal, sig ...os.Signal)
nodejs: process.on('SIGINT', () => {... });remove a handler:
go: func Reset(sig ...os.Signal)
go: func Stop(c chan<- os.Signal)ignore signal:
go: func Ignore(sig ...os.Signal)More in depth for go: When the process starts it will register a handler for almost all signals (except SIGCONT, SIGTSTP, SIGTTIN, SIGTTOU). A signal handler is not set for SIGHUP and SIGINT if those are ignored. For the pthread signals, an alternative stack is registered to avoid stack-overflow problem in case a go-stack is used. Signals for the runtime to work are unblocked when a thread is created (SIGILL, SIGTRAP, SIGBUS, SIGFPE, SIGSEGV, SIGSTKFLT, pthread signals, SIGCHLD, SIGPROF). Only the last two may be handled by the user. Several signals will cause the application to stop when not handled by the user: SIGHUP, SIGINT, SIGQUIT, SIGABRT, SIGTERM. The remaining signals can be handled by the user but won't cause the application to stop when unhandled. New Process When a new process is created (fork,execve) it inherits the thread sigmask and the process ignored&default signals. The go implementation is not providing more control over this. The .NET Core implementation is resetting the signal mask keeps the ignored signals.
stephentoub
commented
7 years ago @eyalsk, @jnm2, @tmds, great suggestions and info, thanks!
One of the things we've talked about doing is having a System.Posix (https://github.com/dotnet/corefx/issues/3186#issuecomment-139818980), akin to Mono's Mono.Posix, which provides safe P/Invoke entrypoints for much of POSIX (or at least the parts of POSIX that could be done safely, e.g. relevant to this thread, I don't think we would have a wrapper for signal or sigaction, as it couldn't be done safely with the same semantics as what POSIX documents). If we were to do that, then I'm envisioning potentially a two-layer approach here for sending signals. At the lower layer, you'd have a Unix-specific API, e.g.
// assembly System.Posix.dll // I'm making up a name, could be something else
namespace System.Posix
{
public enum SignalNumber : int {
SIGHUP = 1,
SIGINT = 2,
SIGQUIT = 3,
...
}
public static class Functions
{
public static int Kill(int pid, SignalNumber signum)
{
... // translate signum as is necessary for the specific unix platform, invoke kill syscall
}
}
...
}Code specific to Unix could use this; we could choose to make the assembly available on Windows, but the Windows implementation would just throw PlatformNotSupportedException. This seems in line with what these other platforms do for sending signals (per @tmds's and @jnm2's links and comments), as pretty much all of them seem to do this with a signature that maps 1:1 with kill, and as a quick look at the docs suggest that, since signals aren't a Windows concept, the low-level functions for interacting with them aren't available. In some cases we may be able to emulate them, but I'm not convinced that's a good idea at this level.
Then there's the question of writing a higher-level API that does try to smooth over the cross-platform boundaries. It looks like some of the other platforms put such support on the corresponding process class, and that could make sense for .NET as well. We already have Process.Kill, which on Windows calls TerminateProcess and on Unix calls kill with SIGKILL. We could add methods for other operations which are representable as signals on Unix and which have a corresponding implementation on Windows, e.g. have an Interrupt method that uses GenerateConsoleCtrlEvent on Windows and uses kill with SIGINT on Unix (https://github.com/dotnet/corefx/issues/1838). I'm not sure how many other signals would make sense in this context, but please share if you have thoughts.
Today each library in corefx that needs to P/Invoke to POSIX ends up P/Invoking to System.Native.so/dylib, which in turn calls the corresponding libc function. If we added such a System.Posix.dll, I'm imagining such libraries would instead make managed calls to functions in System.Posix.dll, which would P/Invoke to System.Native.so/dylib. That would help us to further consolidate all of the P/Invokes we have across the repo, potentially even reducing binary size, and helping with maintenance, in addition to exposing these functions so others don't have to write the P/Invokes themselves.
iam3yal
commented
7 years ago @stephentoub
but the Windows implementation would just throw PlatformNotSupportedException.
Can you elaborate on this? you mean like in cases where the signal doesn't make sense on Windows, right?
Anyway, the approach you want to take seems pretty reasonable to me.
stephentoub
commented
7 years ago Can you elaborate on this? you mean like in cases where the signal doesn't make sense on Windows, right?
I meant for Functions.Kill, regardless of the values passed in to it. The function is specifically about signals in a sense that doesn't exist in Windows, and I'm suggesting at this layer there shouldn't be any attempt at emulation.
iam3yal
commented
7 years ago @stephentoub Yeah, I agree, thanks for the clarification!
tmds
commented
7 years ago @stephentoub I like System.Posix for these reasons: I don't need to fiddle with pinvoke signatures, it will work better on other platform compared to DIY and it will hopefully tackle struct layout issues (struct stat, pthread_t, ...). Some practical concerns. I wonder how you decide what you'll put in there. If it is part of a feature request like this, it makes the API surface you need to talk about bigger. And it will introduce multiple ways of doing the same thing.
Continuing a bit on (3): We can add SendSignal methods on Thread and Process.
void Process.SendSignal(SignalNumber, bool sendToGroup);
void Thread.SendSignal(SignalNumber);For the modeling of SignalNumber. Both go and nodejs are representing these as strings which are then mapped to an integer. SIGRTMAX is a challenging value to represent.
public struct SignalNumber
{
public SignalNumber(int);
public int Value { get; }
public static SignalNumber Hup { get; }
public static SignalNumber Term { get; }
// ...
public static SignalNumber RtMin { get; }
public static SignalNumber RtMax { get; }
// value returned when not present on platform
public static SignalNumber Unsupported { get; }
}Where could this type live?
stephentoub
commented
7 years ago We can add SendSignal methods on Thread and Process
I get the desire to put this there, but I'm not a huge fan of adding new methods like this to Thread or Process, adding new platform-specific methods to types that are generally cross-platform.
If it is part of a feature request like this, it makes the API surface you need to talk about bigger
I'd be ok with us augmenting the type over time, such that we start it with a small number of functions, and then separately add more to it. We'd just need to work through the general guidelines for the type to know that the few APIs we're adding will generally match the rest in style.
SIGRTMAX is a challenging value to represent.
Can you elaborate?
tmds
commented
7 years ago I get the desire to put this there, but I'm not a huge fan of adding new methods like this to Thread or Process, adding new platform-specific methods to types that are generally cross-platform.
I don't have a strong feeling about this. Something like this then?
public class Signal
{
public static void SendTo(Process, SignalNumber, bool sendToGroup);
public static void SendTo(Thread, SignalNumber);
}Or as extension methods?
SIGRTMAX is a challenging value to represent. Can you elaborate?
The realtime signals are a range of platform specific length. So it isn't possible to enumerate them in a enum. That is the rationale to use struct properties instead of enum values. That way you can start at RtMin and go up to RtMax.
tmds
commented
7 years ago @stephentoub and others, unless someone else wants to do it, I can send a PR for the send signal feature. I'd put it in a new 'System.IO.Signals' project. We can make changes based on PR feedback.
stephentoub
commented
7 years ago Thanks, @tmds.
public static void SendTo(Process, SignalNumber, bool sendToGroup);
I don't think we should use Process here either; if the signature takes an int and the developer already has a Process, all they have to do is pass in p.Id, but if the signatures takes a Process and the developer has an int, they have to use the Process class to get a Process object for that int, which is not cheap. And the implementation is just going to turn around and extract the int Id anyway. I think it's better to keep this function lower-level than System.Diagnostics.Process and keep it entirely out of the abstraction.
public static void SendTo(Thread, SignalNumber);
What is the benefit of this? We've already discussed how the signals would be handled asynchronously, so it likely wouldn't end up being handled by the target thread anyway. Even if it was, for certain dispositions it'll affect the whole process anyway.
That way you can start at RtMin and go up to RtMax.
I see. Ok, I can see the value of the struct as you've defined it. It will likely be a bit more expensive, because accessing a property will require a P/Invoke to get the signal number, though it could be cached so that it would only need to be done once.
I can send a PR for the send signal feature. I'd put it in a new 'System.IO.Signals' project. We can make changes based on PR feedback
I would rather get the design ironed out in issues before opening PRs. You're welcome to provide links to branches that include your work, of course, but opening PRs when new APIs haven't been fully reviewed means the PRs will just sit there, and we'll end up with discussions in both issues and on the PR. Thanks, again, for your interest in working on this.
Where could this type live?
Given everything discussed thus far, I still think the posix functions wrappers approach makes the most sense. Thoughts? If we went that route, then such a struct would just live in that library as well, and the Kill method would accept a SignalNumber struct in addition to the int pid.
tmds
commented
7 years ago I don't think we should use Process here either; if the signature takes an int and the developer already has a Process, all they have to do is pass in p.Id, but if the signatures takes a Process and the developer has an int, they have to use the Process class to get a Process object for that int, which is not cheap. And the implementation is just going to turn around and extract the int Id anyway. I think it's better to keep this function lower-level than System.Diagnostics.Process and keep it entirely out of the abstraction.
I don't think it will be common to send signals to processes for which you don't have a Process instance. But I understand the interest in avoiding the overhead of the Process class creation for when you don't.
What is the benefit of this? We've already discussed how the signals would be handled asynchronously, so it likely wouldn't end up being handled by the target thread anyway. Even if it was, for certain dispositions it'll affect the whole process anyway.
It can be used to unblock the target thread on a blocking syscall. You can set a variable to indicate the thread should stop and then send the signal to the thread to unblock it. The disposition of this signal would be a no-op. It is the posix wrapper for pthread_kill. Since the use case is far less common, maybe we leave it out for now.
I see. Ok, I can see the value of the struct as you've defined it. It will likely be a bit more expensive, because accessing a property will require a P/Invoke to get the signal number, though it could be cached so that it would only need to be done once.
Right. As an alternative for the SignalNumber struct, we can also just use an int.
Then we are at:
namespace System.Posix
{
public static class Signal
{
public static void Send(int pid, int signal, bool sendToGroup);
public static int Hup { get; }
public static int Term { get; }
// ...
public static int RtMin { get; }
public static int RtMax { get; }
// value returned when not present on platform
public static int Unsupported { get; }
}
}@tmds
public static void Send(int pid, int signal, bool sendToGroup);
I'd rename the method to SendToProcess, I don't like short names just for the sake of being succinct, besides, say tomorrow there might be a requirement to add a SendToThread that takes the same parameters the name would be at odds, also, I think that it makes the code more readable when the target you're sending a signal to is specified as part of the name so you may want to reconsider it.
Just my 2c.
tmds
commented
7 years ago @stephentoub et al, I made a signal sending implementation, as a proof-of-concept. See https://github.com/tmds/Tmds.Posix.
I experimented a bit with the error handling. The implementation has two methods for sending signals.
public static void SendToProcess(int pid, int signal, bool sendToGroup);
public static NativeError TrySendToProcess(int pid, int signal, bool sendToGroup);The NativeError used here is just wrapping the int errno. It can be turned into something more useful with some extension methods:
public static void ThrowOnError(this NativeError error);
public static Error ToError(this NativeError error);
public static string Describe(this NativeError error);
public static bool IsSuccess(this NativeError error);The Error is a platform agnostic representation of the errno (it is the same as the internal Error enum in core).
Throw will throw a PosixException which exposes the error message, native and platform-independent error number.
iam3yal
commented
7 years ago @tmds
Good job.
However, I'm not sure about this TrySendToProcess method, generally you'd expect a Try method to follow the tester-doer pattern the way it's implemented throughout the framework where these methods generally return a boolean value and have some out parameters so maybe you can rename it to Kill? but maybe @stephentoub will have better ideas. :)
patricksuo
commented
7 years ago @stephentoub @eyalsk @tmds . hello, any progress on this? Can we have this feature in 2.0 ?
karelz
commented
7 years ago @sillyousu 2.0 feature complete / API freeze was on 4/14. This is something we might potentially fund for next version - 2.1, but it is not yet done deal. Are you just interested in availability time, or seeking way of contributing towards this effort?
agc93
commented
7 years ago @karelz any news on whether this is likely to make it to 2.1? Lack of signal handling is a bit of a pain working with .NET Core daemons in mixed environments :confounded:
dlech
commented
7 years ago I though the verdict was that using Mono.Posix was the solution for Unix-specific stuff. https://www.nuget.org/packages/Mono.Posix.NETStandard/1.0.0-beta3
danmoseley
commented
7 years ago @agc93 have you tried Mono.Posix? Is it sufficient?
tjaart
commented
7 years ago Do you know where can I find some example code on using Mono.Posix.NETStandard @dlech ?
eerhardt
commented
7 years ago @tjaart - are you looking for any sample code? Or specifically signal handling?
For signal handling, you have 2 options:
var sigintRaised = new UnixSignal(Signum.SIGINT);
while (sigintRaised.WaitOne()) {
Console.WriteLine(“Control-C pressed!”);
}Note that it blocks the thread until the signal is raised. See http://docs.go-mono.com/?link=T%3aMono.Unix.UnixSignal for docs.
dlech
commented
7 years ago Do you know where can I find some example code on using Mono.Posix.NETStandard @dlech ?
Don't know of any examples. I've just used the documentation as a guide.
deinok
commented
6 years ago @karelz Any progress on this? Also seeking a way of contributing if required.
karelz
commented
6 years ago It is up-for-grabs and marked for Future - i.e. no immediate plans from our side at this moment.
However, I think there was some work done recently in the area by @tmds or @eerhardt. @danmosemsft likely knows more about our plans/prioritization in the area for 2.1/Future.
Also, did you try Mono.Posix as suggested above? Is it not sufficient?
deinok
commented
6 years ago @karelz Yes, Mono.Posix match the requirements, only asking if the intention of creating System.Posix.Signal is in development or if I can start an implementation for a future PR.
Signals are a ubiquitous mechanism across Unix systems for interactions between processes and the kernel. The only API we currently have for sending any signal is Process.Kill(), which sends a SIGKILL signal, but to write robust Unix applications (and services) you likely want to be able to both send signals and handle specific signals. The former can be done by a dev using a P/Invoke, but the latter would really require libcoreclr support to do correctly, especially since the runtime itself already hooks certain signals (e.g. a SIGINT signal sent by ctrl-C, which we then subscribe to the runtime for in Console.CancelEventHandler).