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

signal module ignores external signal changes #57494

Open
vilya mannequin opened this issue Oct 28, 2011 · 18 comments
Open

signal module ignores external signal changes #57494

vilya mannequin opened this issue Oct 28, 2011 · 18 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@vilya
Copy link
Mannequin

vilya mannequin commented Oct 28, 2011

BPO 13285
Nosy @pitrou, @vstinner, @bitdancer, @njsmith, @florentx, @takluyver, @akheron, @vadmium, @jdemeyer

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2011-10-28.11:27:57.961>
labels = ['type-bug', 'library']
title = 'signal module ignores external signal changes'
updated_at = <Date 2019-01-16.17:43:44.206>
user = 'https://bugs.python.org/vilya'

bugs.python.org fields:

activity = <Date 2019-01-16.17:43:44.206>
actor = 'jdemeyer'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2011-10-28.11:27:57.961>
creator = 'vilya'
dependencies = []
files = []
hgrepos = []
issue_num = 13285
keywords = []
message_count = 18.0
messages = ['146553', '146560', '146564', '146868', '146872', '146935', '262229', '262247', '285831', '285842', '286264', '286266', '286332', '291529', '291548', '325700', '333765', '333780']
nosy_count = 12.0
nosy_names = ['pitrou', 'vstinner', 'vilya', 'r.david.murray', 'njs', 'flox', 'neologix', 'takluyver', 'petri.lehtinen', 'martin.panter', 'jdemeyer', 'rpcope1']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue13285'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

@vilya
Copy link
Mannequin Author

vilya mannequin commented Oct 28, 2011

The signal module is oblivious to any changes to the set of installed signal handlers which occur outside of the module. This can happen when a native module changes a signal handler, or when the python interpreter is embedded in another program which installs its own signal handlers.

In this case, saving and restoring a signal handler through python doesn't work correctly. For example, if the SIGINT handler is set externally after the signal module is initialised, the following code will replace the external signal handler with python's default_int_handler:

  handler = signal.getsignal(signal.SIGINT)
  signal.signal(signal.SIGINT, handler)

So it's impossible to reliably save and restore signal handlers through python when they can also be changed outside the python interpreter.

Also, if there's a signal handler installed before the module is initialised, signal.getsignal() will return None for it - making it impossible to restore the correct handler after disabling it.

The reason is that the signal module only checks for existing handlers when it's initialised. The results get stored in the Handlers array, which is then used by all subsequent calls to signal.getsignal(). There are no further checks to see whether the native signal handlers have changed.

@vilya vilya mannequin added the type-bug An unexpected behavior, bug, or error label Oct 28, 2011
@vilya vilya mannequin changed the title signal module in ignores external signal changes signal module ignores external signal changes Oct 28, 2011
@neologix
Copy link
Mannequin

neologix mannequin commented Oct 28, 2011

So it's impossible to reliably save and restore signal handlers through
python when they can also be changed outside the python interpreter.

signal.getsignal() or signal.signal() return the current/previous handler as a Python function. How could it return a reference to a native (i.e. C) signal handler?
While we could in theory return it as a magic cookie (i.e. the handler's address as returned by sigaction/signal) that can just be passed back to signal.signal(), it would be a bad idea: if the user passes an invalid address, the process will crash when the signal is received.

@vilya
Copy link
Mannequin Author

vilya mannequin commented Oct 28, 2011

Could it return an opaque wrapper object, rather than just the raw address? Something like:

typedef struct _PyNativeSignalHandler {
  PyObject_HEAD
  sighandler_t handler_func;
} PyNativeSignalHandler;

where the type object doesn't expose any way to read or manipulate the handler_func. Would that work, do you think?

@florentx florentx mannequin added the stdlib Python modules in the Lib dir label Oct 28, 2011
@akheron
Copy link
Member

akheron commented Nov 2, 2011

Could it return an opaque wrapper object, rather than just the raw address? Something like:

Are you suggesting that it would return either a Python function object or a wrapper object?

@neologix
Copy link
Mannequin

neologix mannequin commented Nov 2, 2011

> Could it return an opaque wrapper object, rather than just the raw
> address? Something like:

Are you suggesting that it would return either a Python function object or a
wrapper object?

I think it would be an ugly kludge. I don't like the idea of passing
around an opaque "blob" just for that purpose (I mean, it's really a
corner case).
So I'm -1.

@vilya
Copy link
Mannequin Author

vilya mannequin commented Nov 3, 2011

Petri: yes, that what I was suggesting.

Charles-François: I'm certainly open to alternatives. Unless I've overlooked something though, the problem is that no workaround is possible at the moment. BTW this came up in the context of a customer script for the software I work on, so it's not just a theoretical problem - at least, not for me. :-)

I guess another solution would be methods to save and restore the native signal handers, e.g. savesignal(signum) & restoresignal(signum). That wouldn't entirely solve the problem - if someone called setsignal() before calling savesignal(), they'd end up in the same situation - but it would at least permit a workaround.

@vadmium
Copy link
Member

vadmium commented Mar 23, 2016

Also, the documentation currently suggests it returns None in these cases, but it actually returns SIG_DFL in at least one case (noticed in bpo-23735).

FWIW Vilya’s opaque object sounds fairly sensible to me. Yes, it is ugly, but one does not look at Unix signals expecting to find beauty :)

@vstinner
Copy link
Member

If anyone wants to work on this issue, I suggest to requalify it as a documentation issue. Just explain the behaviour and don't try to implement complex workaround.

@takluyver
Copy link
Mannequin

takluyver mannequin commented Jan 19, 2017

I'd like to make the case for a fix in the code again. Our use case is, I believe, the same as Vilya's. We want to temporarily set a signal handler from Python and then restore the previous handler. This is fairly straightforward for Python handler functions, and SIG_DFL and SIG_IGN, but it breaks if anything has set a C level signal handler.

The opaque wrapper object is a solution that had occurred to me too. Another option would be a context manager implemented in C (I assume context managers can be written in C) which can set one or more signal handlers on entry, and restore them on exit.

@bitdancer
Copy link
Member

IMO the signal handler context manager would be useful (I have existing code where I wrote one that didn't quite work right :). I suggest you propose this on python-ideas.

@jdemeyer
Copy link
Contributor

Let me add that this "low-level opaque object" would be rather easy to implement on POSIX systems (I have no clue about other systems such as Windows). I could implement it, but it would be good to have some pre-approval from Python devs that it's a good idea to do this.

@takluyver
Copy link
Mannequin

takluyver mannequin commented Jan 25, 2017

I pitched both the opaque handle and the context manager to python-ideas, but didn't get any responses, positive or negative.

@jdemeyer
Copy link
Contributor

Here is a proposal for an API:

  • getsignal: return the Python-level signal handler (this is an existing function)

  • setsignal: set the Python-level signal handler (but not the OS-level signal handler)

  • getossignal: get the OS-level signal handler as opaque object

  • setossignal: set the OS-level signal handler with an opaque object

Using these primitives, you could implement anything that you want on a higher level.

@jdemeyer
Copy link
Contributor

Ping? I'll try to implement this in cysignals (which is more difficult than doing it in CPython because I cannot access all internals of the Python signal module).

@jdemeyer
Copy link
Contributor

I have a preliminary implementation (in the cysignals package) at sagemath/cysignals#53

@njsmith
Copy link
Contributor

njsmith commented Sep 19, 2018

Here's another case where this bug bites us: python-trio/trio#673

At startup, Trio checks if SIGINT is currently being handled by Python's default SIGINT handler, and if so it substitutes its own SIGINT handler (which works just like the default SIGINT handler, except with some added magic [1]).

However, this user has a C library that installs its own handler for SIGINT. When this happens, the Python signal.getsignal() function returns stale, incorrect information (claiming that the Python signal handler is still working, even though it isn't), and this causes Trio to do the wrong thing.

Vilya's "magic cookie" approach above is the one that I was going to suggest before I saw this bug :-).

Jeroen's version seems more complicated than necessary to me, and also it doesn't seem to work for my case: I need to check what the current signal handler is and make some decision based on that result. In Jeroen's API, I can see what the Python-level signal handler is, but there's no way to find out whether that signal handler is actually in use or not. Instead, we should make signal.getsignal() do something like:

  c_handler = PyOS_getsig(signalnum);
  if c_handler == the_python_signal_handler:
      # Python is handling this signal; return the Python-level handler
      return Handlers[signalnum].func
  elif c_handler in [SIG_DFL, SIG_IGN]:
      return c_handler
  else:
      return OpaqueCookie(c_handler)

[1] https://vorpus.org/blog/control-c-handling-in-python-and-trio/

@jdemeyer
Copy link
Contributor

In Jeroen's API, I can see what the Python-level signal handler is, but there's no way to find out whether that signal handler is actually in use or not.

I added support for that in the latest cysignals release. Now you can do

>>> import signal
>>> from cysignals.pysignals import getossignal, python_os_handler
>>> _ = signal.signal(signal.SIGINT, signal.default_int_handler)
>>> getossignal(signal.SIGINT) == python_os_handler
True

Note that cysignals is POSIX-only for now (it assumes sigaction), but the code could easily be ported to other systems. Ideally it would become part of CPython's signal module.

@jdemeyer
Copy link
Contributor

For reference, the sources for my implementation: https://github.com/sagemath/cysignals/blob/master/src/cysignals/pysignals.pyx

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

6 participants