sagemath / cysignals

cysignals: interrupt and signal handling for Cython. Source repository for https://pypi.org/project/cysignals/
GNU Lesser General Public License v3.0
43 stars 23 forks source link

Add context manager for saving / restoring all signal handlers #101

Closed embray closed 5 years ago

embray commented 5 years ago

Unless I'm missing something there isn't anything exactly like this yet in cysignals. There is cysignals.pysignals.changesignal, but I would like something like that either for all signals (by default), or optionally a subset. It doesn't necessarily need to change the existing signal handlers either; just save/restore them, though perhaps there could be an option to set them as well (e.g. to SIG_DFL or SIG_IGN).

In Sage we already do something similar to this with direct sigaction calls in sage.libs.ecl.init_ecl(). But it would be better to have a generalized version of this as a context manager.

jdemeyer commented 5 years ago

Not saying that this is a bad idea, but what is your use case?

embray commented 5 years ago

I sort of implied one already: in Sage's libecl interface there is already code much like this that could easily be replaced with a nice helper from cysignals. Of course, that's just one case. For the libgap interface this would also be useful. I think it might also be useful for ppl, and some other libraries interfaced from Sage, that unconditionally install their own signal handlers that we might like to just do away with, or at least wrap with our own.

jdemeyer commented 5 years ago

The Sage code for ECL assumes that there are 31 relevant signals, numbered 1 to 31. I'm not sure where that number comes from and whether it's a universal constant.

But under that assumption, it should be possible to implement.

embray commented 5 years ago

There are 32 listed here: https://en.wikipedia.org/wiki/Signal_(IPC)#POSIX_signals

Such a utility could also take an optional argument or arguments to modify the default signum list, but just listing all these signals (and by their macros rather than numeric values) as the default list to save/restore might be best.

embray commented 5 years ago

I also have encountered SIGEMT and SIGPWR before but I don't think anything we care about would use those :)

jdemeyer commented 5 years ago

I also have encountered SIGEMT and SIGPWR before but I don't think anything we care about would use those :)

I vaguely remember some Sage package that would use SIGPWR internally for some reason (it might have been ECL actually). Then Sage would crash with the message

Power Failure
jdemeyer commented 5 years ago

listing all these signals (and by their macros rather than numeric values)

That would just be a mess with lots of #ifdefs and then we potentially need to worry about duplication (various systems have multiple names for the same signal). Using the range from 1 to 32 is simple and seems to work for Sage's ECL wrapper.

vbraun commented 5 years ago
$ kill -l
 1) SIGHUP   2) SIGINT   3) SIGQUIT  4) SIGILL   5) SIGTRAP
 6) SIGABRT  7) SIGBUS   8) SIGFPE   9) SIGKILL 10) SIGUSR1
11) SIGSEGV 12) SIGUSR2 13) SIGPIPE 14) SIGALRM 15) SIGTERM
16) SIGSTKFLT   17) SIGCHLD 18) SIGCONT 19) SIGSTOP 20) SIGTSTP
21) SIGTTIN 22) SIGTTOU 23) SIGURG  24) SIGXCPU 25) SIGXFSZ
26) SIGVTALRM   27) SIGPROF 28) SIGWINCH    29) SIGIO   30) SIGPWR
31) SIGSYS  34) SIGRTMIN    35) SIGRTMIN+1  36) SIGRTMIN+2  37) SIGRTMIN+3
38) SIGRTMIN+4  39) SIGRTMIN+5  40) SIGRTMIN+6  41) SIGRTMIN+7  42) SIGRTMIN+8
43) SIGRTMIN+9  44) SIGRTMIN+10 45) SIGRTMIN+11 46) SIGRTMIN+12 47) SIGRTMIN+13
48) SIGRTMIN+14 49) SIGRTMIN+15 50) SIGRTMAX-14 51) SIGRTMAX-13 52) SIGRTMAX-12
53) SIGRTMAX-11 54) SIGRTMAX-10 55) SIGRTMAX-9  56) SIGRTMAX-8  57) SIGRTMAX-7
58) SIGRTMAX-6  59) SIGRTMAX-5  60) SIGRTMAX-4  61) SIGRTMAX-3  62) SIGRTMAX-2
63) SIGRTMAX-1  64) SIGRTMAX    

Note that the enumeration is not contiguous (e.g. no 32). Doing something with #32 is probably bad since its used internally for multithreading on linux. As already mentioned, the range [1, 31] is as portable as ecl but obviously not a standard.

jdemeyer commented 5 years ago

Also Python seems to use the range 1-31, so I will go with that.

embray commented 5 years ago

I vaguely remember some Sage package that would use SIGPWR internally for some reason

Hah, I take it back then.

jdemeyer commented 5 years ago

Done in master.

embray commented 5 years ago

Thanks. Blocking them looks like it was a good idea too.