Skip to content

Python doesn't support real time signals #56269

Closed

Description

@vstinner
BPO 12060
Nosy @gpshead, @pitrou, @vstinner
Files
  • rt_signal.patch
  • example.py
  • signal_pedantic.diff
  • 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 = <Date 2011-05-14.23:26:30.120>
    created_at = <Date 2011-05-11.21:57:41.702>
    labels = ['interpreter-core']
    title = "Python doesn't support real time signals"
    updated_at = <Date 2011-05-15.08:28:43.492>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2011-05-15.08:28:43.492>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2011-05-14.23:26:30.120>
    closer = 'gregory.p.smith'
    components = ['Interpreter Core']
    creation = <Date 2011-05-11.21:57:41.702>
    creator = 'vstinner'
    dependencies = []
    files = ['21976', '21977', '21989']
    hgrepos = []
    issue_num = 12060
    keywords = ['patch']
    message_count = 15.0
    messages = ['135808', '135809', '135825', '135827', '135828', '135829', '135832', '135871', '135879', '135885', '136001', '136004', '136005', '136021', '136023']
    nosy_count = 7.0
    nosy_names = ['gregory.p.smith', 'pitrou', 'vstinner', 'nadeem.vawda', 'neologix', 'sdaoden', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue12060'
    versions = ['Python 3.3']

    Activity

    1. vstinner commented on May 11, 2011

      @vstinner
      MemberAuthor

      If a real time signal is raised 2 times whereas the signal is blocked, unblock the signal will call the signal handler twice. The C signal handler of the Python signal module only stores a boolean to say if the Python signal handler should be called or not in Py_CheckSignals().

      If the C signal handler is called twice, the Python signal handler is only called once.

      Attached patch is a draft to fix this issue. The patch is not completly safe.

    2. vstinner commented on May 11, 2011

      @vstinner
      MemberAuthor

      example.py: example to demonstrate the problem. The Python signal handler is only called once, it should be called twice.

    3. sdaoden commented on May 12, 2011

      sdaodenmannequin
      Mannequin

      Dunno.

      The patch is not completely safe.

      Yeah it will not work without atomic ops.
      Unfortunately the C standart seems to go into a direction
      noone understands - as if a atomic_compare_and_swap() would
      not suffice! Do you know any machine language which reflects
      what that standart draft describes? I don't.

      The NSIG detection of Modules/signalmodule.c uses 64 as a fallback.
      32 seems to be more reasonable.
      And you test against it instead of RTMAX in the patch.
      (signalmodule.c also exports Python constants RTMIN and RTMAX
      even though the standart explicitely allows these values to
      be non-constants;
      http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html; last time i've done anything on signals - in 2005 - that was
      used nowhere - Linux, FreeBSD - though.)

      Often there is a huge whole in between NSIG and RTMIN, but
      struct Handlers is 8 or 12 bytes (unless the compiler does the
      alignment - ouuh), so 32 unused members in Handlers[] will not
      cost the world anyway; on Mac OS X (no RTSIG support?!? ;)
      Python is at least 6 megabytes of memory anyway.

      And does anyone actually know why the last time i looked after this
      (on Linux, then) realtime signals had a default action EQ SIGABRT?
      Armchair crouchers...

    4. pitrou commented on May 12, 2011

      @pitrou
      Member

      Is it a theoretical concern or does it affect real software?

    5. vstinner commented on May 12, 2011

      @vstinner
      MemberAuthor

      Is it a theoretical concern or does it affect real software?

      It's more theoretical.

    6. vstinner commented on May 12, 2011

      @vstinner
      MemberAuthor

      Yeah it will not work without atomic ops.

      ... Or we can maybe block the signals (all signals or just one signal?) using pthread_sigmask(SIG_BLOCK) while we access the Handlers array. But pthread_sigmask() is not available on all OSes.

      On my Linux box, Python 3.3 says that signal.NSIG is equal to 65 which looks correct.

      does anyone actually know why the last time i looked after this
      (on Linux, then) realtime signals had a default action EQ SIGABRT?

      The manpage says "The default action for an unhandled real-time signal is to terminate the receiving process." It is an arbitrary choice. Why do you care about the default action?

      The NSIG detection of Modules/signalmodule.c uses 64 as a fallback.
      32 seems to be more reasonable.
      And you test against it instead of RTMAX in the patch.

      I don't understand: I don't use RTMAX in my patch.

    7. sdaoden commented on May 12, 2011

      sdaodenmannequin
      Mannequin

      On my Linux box, Python 3.3 says that signal.NSIG is equal to 65
      which looks correct.

      On FreeBSD NSIG only counts "old" signals (32, one 32 bit mask),
      SIGRTMIN is 65 and SIGRTMAX is 126.
      Our internal old signal.h states

          * If we do have realtime signals, #rtmin is 35 (i.e.,
          * #nsig, FreeBSD+) or something like 38 or even 40 (Linux),
          * and #rtmax is most likely 64 (Linux) or 128 (FreeBSD+).
      

      so that this seems to be somewhat constant in time.
      (#rtmin: we take some of those RT sigs for internal purposes if
      possible. This was maybe a bad and expensive design decision.)

      Why do you care about the default action?

      • \brief Hooking program crashes (\psa crash.h crash.h\epsa).
      • \note
      • Installed hooks (normally) execute from within an internal
      • signal handler!

      So many syscalls for things which don't matter almost ever.
      And that may even cost context-switches sometimes.

      I don't understand: I don't use RTMAX in my patch.

      + for (signum = 1; signum < NSIG; signum++) {

      This will not catch the extended signal range on FreeBSD.

    8. neologix commented on May 12, 2011

      neologixmannequin
      Mannequin

      If the C signal handler is called twice, the Python signal handler is only called once.

      It's not the only shortage with the current implementation regarding (real-time) signals. Another one is that they're delivered out-of-order (lowest-numbered signal first), and the most important one - especially for real-time signals - is that the handlers are executed synchronously, when Py_CheckSignals() is called.
      While we can't do much about the the later, there's at least a way to handle the other issues, a small variant of the self-pipe trick:

      • in the signal module initialization code, create a pipe with both ends set to non-blocking
      • in the signal handler, write the signal received to the pipe (just like what's done for wakeup_fd)
      • in Py_CheckSignals(), read from the pipe: the signals will be read in order, as many times as they were received. Call the handler for each signal read from the pipe.

      advantages:

      • it's async-safe
      • signals are handled in order
      • signal handlers are called the correct number of times
      • you don't have to walk through the whole array of signal handlers, since you know exatly what signals have been received

      drawbacks:

      • might be a bit slower, because of the read syscall
      • consumes 2 FDs
      • have to reinit the pipe on fork
      • does Windows have pipe/read/write?
      • maybe overkill

      But I'm really not sure that fixing this is worth it...

      By the way, to be pedantic, in the current code, wakeup_fd and Handlers should be volatile, and tripped should be sig_atomic_t.

    9. vstinner commented on May 12, 2011

      @vstinner
      MemberAuthor

      It's not the only shortage with the current implementation regarding
      (real-time) signals. Another one is that they're delivered
      out-of-order (lowest-numbered signal first),

      Oh yes, it can be a problem.

      and the most important one - especially for real-time signals - is
      that the handlers are executed synchronously, when Py_CheckSignals()
      is called.

      Evaluate Python code in a signal handler is really not a good idea! And
      because of the GIL, I don't think that we can do better. But with the
      GIL of Python 3.3, the Python signal handler will be called "immediatly"
      before the next instruction, instead of having to wait something like
      100 Python instructions (sys.getcheckinterval()). On this point, Python
      3.3 is better than all previous versions.

      While we can't do much about the the later, there's at least a way to
      handle the other issues, a small variant of the self-pipe trick:
      (...)
      drawbacks:

      • might be a bit slower, because of the read syscall
      • consumes 2 FDs

      Consume 2 FDs can be surprising. We cannot do that by default (just to
      have in-order or real time signals).

      • have to reinit the pipe on fork

      Oh yes, it can be a problem.

      • does Windows have pipe/read/write?

      Do you really think that Windows supports real time signals? :) Windows
      has pipes: see the Connection object of the multiprocessing module
      (Antoine is working on this topic). But I don't remember if they can be
      used as POSIX files (using file descriptors).

      • maybe overkill

      But I'm really not sure that fixing this is worth it...

      It looks like your reimplements the whole signal machinery between the
      kernel and the application... But anyway, we need something between the
      C signal handler and the Python signal handler (because we cannot
      execute Python code in a C signal handler).

      Well, we can imagine to have an option/function to enable real time
      and/or in-order signals (e.g. using pipes).

      Can we call this mode "real time"? Real time means that we warranty a
      maximum response time.

      By the way, to be pedantic, in the current code, wakeup_fd and
      Handlers should be volatile, and tripped should be sig_atomic_t.

      Yes. Can you please write a patch for this?

    10. neologix commented on May 12, 2011

      neologixmannequin
      Mannequin

      Evaluate Python code in a signal handler is really not a good idea!

      I know, you're limited to async-safe functions, among other things :-)

      because of the GIL, I don't think that we can do better. But with the
      GIL of Python 3.3, the Python signal handler will be called "immediatly"
      before the next instruction, instead of having to wait something like
      100 Python instructions (sys.getcheckinterval()). On this point, Python
      3.3 is better than all previous versions.

      Nice.

      Well, we can imagine to have an option/function to enable real time
      and/or in-order signals (e.g. using pipes).

      Can we call this mode "real time"? Real time means that we warranty a
      maximum response time.

      Well, I think that we should either make this the default behaviour
      (that's why I was asking whether it's possible on Windows), or not
      implement it. Having two modes for signal handling is confusing.
      But given that this behaviour has never been noticed before and that I
      doubt that people needing real-time signals use Python (and
      vice-versa), maybe we should just stick with the current
      implementation, which is safe and simple (see Antoine's question, "Is
      it a theoretical concern or does it affect real software?").
      On the other hand, if it turns out to be interesting to some people,
      I'd be happy to work on a patch.

      > By the way, to be pedantic, in the current code, wakeup_fd and
      > Handlers should be volatile, and tripped should be sig_atomic_t.

      Yes. Can you please write a patch for this?

      Patch attached.

    11. gpshead commented on May 14, 2011

      @gpshead
      Member

      if you used the pipe approach you'd need to deal with the case of the write blocking (or failing if nonblocking) when the pipe buffer is full. also you'd need to block signals around a fork and reinitialize the pipe in the child before reenabling signals.

      i think all of this is overkill. as said, this isn't a real problem.

    12. neologix commented on May 14, 2011

      neologixmannequin
      Mannequin

      if you used the pipe approach you'd need to deal with the case of the
      write blocking (or failing if nonblocking) when the pipe buffer is full.

      Well, a pipe is 64K on Linux (4K on older kernels). Given that each signal received consumes one byte, I'd be pretty surprised if we managed to fill the pipe before Py_CheckSignals() gets called.

      i think all of this is overkill. as said, this isn't a real problem.

      Definitely. Do you think this can be closed as "wont fix" ?

    13. gpshead commented on May 14, 2011

      @gpshead
      Member

      if someone comes up with a situation where this is a real problem, feel free to reopen it.

    14. python-dev commented on May 15, 2011

      python-devmannequin
      Mannequin

      New changeset 945ca78c38b1 by Victor Stinner in branch '3.1':
      Issue bpo-12060: Use sig_atomic_t type and volatile keyword in the signal module.
      http://hg.python.org/cpython/rev/945ca78c38b1

      New changeset b74999f561ca by Victor Stinner in branch '3.2':
      (Merge 3.1) Issue bpo-12060: Use sig_atomic_t type and volatile keyword in the
      http://hg.python.org/cpython/rev/b74999f561ca

      New changeset 1aa48391da30 by Victor Stinner in branch 'default':
      (Merge 3.2) Issue bpo-12060: Use sig_atomic_t type and volatile keyword in the
      http://hg.python.org/cpython/rev/1aa48391da30

    15. vstinner commented on May 15, 2011

      @vstinner
      MemberAuthor

      if someone comes up with a situation where this is a real problem,
      feel free to reopen it.

      Ok I agree, no problem. But I commited at least Charles's patch ;-)

    16. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      interpreter-core(Objects, Python, Grammar, and Parser dirs)

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions