-
Notifications
You must be signed in to change notification settings - Fork 76
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
Utility to chain signal handlers #78
Comments
Hello I'm not sure I understand your motivation or what exactly you want to do here and how it fits into what signal-hook is supposed to be. But I'll try two guesses. If I'm wrong, please explain what you mean, maybe with an example situation or specific use case you're solving and where signal-hook doesn't work for you :-) Using the stack as an internal backend mechanismThis is already being done, in a form. The previous signal is chained and called after us and we kind of assume whoever registers after us would do the same. This is a mechanism to live in the same program with other libraries that play with signals and not kill each other. However, the „return a flag if it was handled“ is not present, for three reasons:
Furthermore, one can't put the default handler onto a stack or call it because the default handler is not really a function. Exposing such API to usersI believe such approach would go against the goals of the library, in particular these two:
Being able to remove the handlers only in reverse order of their registration seems very limiting, considering the goal is for libraries to be able to register and unregister their interest without any interaction with other libraries doing so. Currently unregistering can be done in arbitrary order. Furthermore, such a big change seems to be in magnitude of „throw it all out and start from scratch“ and then there would be the question why it should be done inside the same library and not a separate one. In particular, this would be a big breaking change to Why not go back to default signal handlerThere are two reasons why signal hook doesn't want to ever go back to the default signal handler (and recommends the user to emulate the behaviour if desired).
(The above talks about rolling back to default ones implicitly under some conditions; I've thought about being able to do it explicitly, pausing all signals in the application for a while and then being able to return them back, but that sounded fragile too, considering there's the option to just emulate them) Is this library the right place?I slightly suspect that what you're trying to answer here is „how does an application decide what to do with a signal“. This assumes that it's the application deciding somewhere on the top level (which is traditionally the necessity with signals because of their limitations, but signal-hook tries to challenge that dogma). Signal-hook, on the other hand, tries to answer „How does multiple independent parts of a program not kill each other when dealing with signals“ (think something like a global memory allocator, instead each part of application playing with the If I'm right in this guess, I believe the right approach would be not to modify signal-hook, but to add an additional layer, the distribution stack. That one would register into signal-hook somehow (using one of the abstractions or maybe go directly to signal-hook-registry) and let the application add or remove these handlers. Any independent library would still get its copy of the notification, because there's nothing known about that library, or if you could ensure there's nobody else dealing with signals, it would act exactly as you describe. I'm not sure this belongs inside signal-hook, or as a separate library (I could imagine both, depending on how it turns out, in the latter case it would make sense to link to the library from signal-hook's docs). But if you think this is something worth exploring, I'm open to discussing how it would look like API wise. |
Even in this scenario, my proposed behaviour makes more sense: just because a library listens for SIGHUP shouldn't change the behaviour of the application. The library should install a signal handler that does its thing and then always returns If the application wants to avoid terminating in response to SIGHUP, then it should install its own signal handler at the very start which just returns
Yeah you have to just re-raise the signal if the previous signal handler was
You don't have to do this: the library would support unregistering in any order, it's just following the stack model would be more usual.
Fair enough. The main issue I have with signal-hook is that it changes the default behaviour even when no signal handlers are attached. If I attach a SIGTERM handler via signal-hook and then remove it, the SIGTERM is forever blocked. That's absolutely not what I want. The stack model naturally solves this because if no handler explicitly intercepts the signal then the default will always be called. When signal-hook is in the same program I can't rely on that, even when using the stack model myself.
Fair enough: I think this can work for listening to signals, but suppressing the default behaviour of signals is fundamentally a global behaviour, and cannot be managed independently. Also: signal-hook will call the previous hook, but only if it's not the default, so replacing the default hook with one that does the same thing changes the behaviour of the program.
This would make more sense if you want to stick with the model of "all the handlers get called". Then suppressing the default/previous handler would be a separate call. |
You have a point there. The removal of the default is a bit of an exception in how it behaves now. The way I've always looked at the existence of default handlers is they are kind of exception/weird and exist only for applications that don't want to touch signal handling. Once one starts doing something about them, they should be taken over completely. But you're right that the current implicit takeover has its set of quirks. I still don't like the full stack model, for the reasons I've mentioned, I'd prefer to treat all the hooks equally and the need of the application code to be inside the low-level hook (therefore unsafe for everyone) feels as a show stopper. And there's backwards compatibility to consider. So let's come up with some way that would allow you to build what you want. I'll throw some ideas here and no, they are not final: Explicit signal takeoverCurrently, the signal is taken over implicitly, on first use. We could add some kind of The Emulation of the default handlerCurrently, there are docs, but maybe a function The stackIs there a reason why that manual stack thing couldn't contain the default handler emulation on the bottom of the stack? How much does that differ from what you have in mind? Current optionsWithout any changes to signal-hook, I think you could register the conditional shutdown and control if its active by a flag. Not exactly what you want, but something that's already there. |
Yeah, explicit initializing would solve the problem I think. You'd have to make it an error to call it more than once. The stack model is simply a way to assign a sensible behaviour to calling this multiple times, but it's not necessary.
Yeah I looked into this a fair amount, and it's annoyingly difficult. These are the options I have:
No: this would be an implementation detail, completely indistinguishable to the user. The default handler should effectively be the bottom of the stack, so there's no special case to consider of an empty stack. |
Either that, or make the call idempotent (at least as far as the replacement of the low-level signal handler goes, it would be possible to still change some of the internal settings by further calls). But I think making it an error is simpler API. Anyway, while thinking about it, I worry about one thing. Making it explicit for something like SIGTERM makes sense, but what about signals that have a default handler that do nothing, in partigular SIGCHLD? I know at least 2 async runtimes now use signal hook behind the scene to handle SIGCHLD for getting notified about asynchronously running processes and that is supposed to „just work“. I think nobody would be happier about having to allow the takeover of SIGCHLD. So what I'm thinking about, it should still allow for the configuration, but in case of the ones that don't do anything by default, implicit initialization should still stay a possibility.
I'm inclined to 1. in here. Looking through documentation, POSIX says what the default handlers should do and there are only like 3 or 4 options. Furthermore, if there's a bug, someone will notice fast and report it. All the others are prone to having some subtle bugs that happen once in a blue moon, and I don't particularly like spin locks that could happen out of the blue inside any part of the program (though people would do better if they masked the signals inside their realtime threads anyway).
I was just checking this is implementable, not if the default handler should or should not be visible to users. Anyway, I was wondering if this could be created in more generic way. Some way o callback chain stack data structure might be useful for other purposes, not just signals, with interface something like: enum ContinuationOp<R> { Stop<R>, Continue };
struct DispatchStack<P, R> { .. }
impl DispatchStack<P> {
fn push(&mut self, cback: Box<FnMut(P) -> R>);
fn pop(&mut self);
fn dispatch(&mut self, param: P) -> Option<R>);
} |
AIUI, there are 4 options:
1 + 3 are easy enough to implement. The others I'm unsure how you'd implement. This is an extract from my attempt to implement a stack-based signal model: unsafe fn delegate(&self, signum: libc::c_int, data: Self::Data) {
if self.0.sa_sigaction == libc::SIG_DFL {
// Default handler. We want to re-raise the signal, but doing so is racy,
// so avoid doing it if the default action is to do nothing anyway. If the
// default action is to terminate the process, then it doesn't matter if
// restoring the original handler is racy since we won't get to that part...
//
// If the default action is to pause the process, then the race condition may
// be a problem when the process is resumed. There's not a huge amount we can
// do about that though...
match signum {
libc::SIGCHLD | libc::SIGCONT | libc::SIGURG | libc::SIGWINCH => {}
_ => {
let prev = self.install(signum);
libc::raise(signum);
if prev.install(signum).0.sa_sigaction != self.0.sa_sigaction {
// Uh oh... Race condition! Just set our signal handler again.
Self::ours().install(signum);
}
}
}
} else if self.0.sa_sigaction != libc::SIG_IGN {
// Non-default handler, call directly
if self.0.sa_flags & libc::SA_SIGINFO != 0 {
mem::transmute::<_, SigActionPtr>(self.0.sa_sigaction)(signum, data.0, data.1);
} else {
mem::transmute::<_, SigHandlerPtr>(self.0.sa_sigaction)(signum);
}
}
} |
For 2, I raise SIGSTOP. That one is one of the unchangeable signals, so it's guaranteed to be at the default. For 3, calling For 4, calling libc::abort should work ‒ that one tries really hard to terminate the program even if SIGABRT is changed (but if the intention is to terminate, it could be reset beforehand anyway). I think I'll split this issue into the two parts, one for the signal initialization, another for that stack (though I'm still unsure if it should be in here). |
I created crates |
It would make more sense to treat signal handlers as a stack.
When you register a signal handler, it gets pushed onto the top of the stack. When you unregister a signal handler, it is popped from the stack.
When a signal is raised, the signal handler at the top of the stack is called. That signal handler can return
true
orfalse
to indicate whether the signal was handled. If the signal was not handled, it is delegated to the next handler in the stack. At the bottom of the stack is the default signal handler.This is how I've seen other pograms use signal handlers, and it is how similar mechanisms work elsewhere, eg. https://docs.microsoft.com/en-us/windows/console/setconsolectrlhandler
I saw in another issue you had concerns with how this crate would be able to unregister its own signal handler, since it wouldn't know which handler to restore. I think that's not necessary: this handler can just always defer to the previous C handler if there are no handlers registered through this crate.
This also solves the problem with restoring the default handler: there is no need, since the default handler will be automatically called if no handler is present to intercept the signal.
The text was updated successfully, but these errors were encountered: