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.set_wakeup_fd does not work unless you register a no-op signal handler for the relevant signal #131803

Open
jfly opened this issue Mar 27, 2025 · 4 comments · May be fixed by #131859
Open
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@jfly
Copy link

jfly commented Mar 27, 2025

Bug report

Bug description:

Consider this program that uses signal.signal.set_wakeup_fd:

demo.py:

import os
import signal
import struct


pipe_r, pipe_w = os.pipe()
os.set_blocking(pipe_w, False)
signal.set_wakeup_fd(pipe_w)

# Conditionally register a signal handler.
if os.environ["REGISTER_SIGNAL_HANDLER"] == "1":
    print("Registering a signal handler for SIGURG")
    signal.signal(signal.SIGURG, lambda signum, frame: None)
else:
    print("Not registering a signal handler for SIGURG")

# Send the signal!
print("Sending myself a SIGURG")
signal.raise_signal(signal.SIGURG)

# Verify that we wrote the signal to the pipe.
print("About to read from pipe_r")
data = os.read(pipe_r, 1)
(raised, ) = struct.unpack('%uB' % len(data), data)
if raised != signal.SIGURG:
    raise Exception("%r != %r" % (raised, signal.SIGURG))

os.close(pipe_r)
os.close(pipe_w)

print("Success!")

If I run this with REGISTER_SIGNAL_HANDLER=1, it works:

$ REGISTER_SIGNAL_HANDLER=1 python demo.py
Registering a signal handler for SIGURG
Sending myself a SIGURG
About to read from pipe_r
Success!

However, if I run this without registering the signal handler, it hangs indefinitely:

$ REGISTER_SIGNAL_HANDLER=0 python demo.py
Not registering a signal handler for SIGURG
Sending myself a SIGURG
About to read from pipe_r

All the real world usages I've been able to find of signal.set_wakeup_fd register a no-op signal handler for the signal(s) they're interested in. For example, see this CPython test:

signal.signal(signal.SIGALRM, handler)
.

I don't know if we would consider this is a bug in the implementation or a bug in the documentation, but IMO, it's a pretty confusing experience for people who don't know this. Some ideas:

  • Change signal.set_wakeup_fd to register a no-op signal handler for you if there isn't a corresponding signal handler already registered.
  • Change signal.set_wakeup_fd to error out if there is no signal handler already registered.
  • Update the docs to mention this requirement: https://docs.python.org/3/library/signal.html#signal.set_wakeup_fd

I'd be happy to implement some of these ideas if folks tell me what seems best!

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

@jfly jfly added the type-bug An unexpected behavior, bug, or error label Mar 27, 2025
@vstinner
Copy link
Member

I don't know if we would consider this is a bug in the implementation or a bug in the documentation, but IMO, it's a pretty confusing experience for people who don't know this.

It's a documentation issue :-) You should register a signal handler in Python to use set_wakeup_fd(), unless Python already has set up a signal handler for the signal.

@jfly
Copy link
Author

jfly commented Mar 28, 2025

Thanks! I'll send in a PR updating the docs.

Python's standard library usually seems more friendly than this. @vstinner what do you think about this suggestion from above?

Change signal.set_wakeup_fd to error out if there is no signal handler already registered.

This function already does other forms of validation (checking if we're in the main thread and checking if the FD is non blocking).

I do understand that that could be considered a breaking change, I don't know what CPython's attitude to this sort of footgun-removal is.

@picnixz picnixz added the extension-modules C modules in the Modules dir label Mar 28, 2025
@NicolasT
Copy link

NicolasT commented Mar 28, 2025

Change signal.set_wakeup_fd to error out if there is no signal handler already registered.

Could you elaborate on what's suggested here? signal.set_wakeup_fd registers the FD to be written to whenever any signal handled by a Python runtime (C) signal handler is delivered (and the C handler gets invoked). How could set_wakeup_fd know which signals you're interested in at all, and hence register a handler for them?

Note there's often at least one signal handler installed already: the runtime installs one for SIGINT (leading to KeyboardInterrupts by default).

@jfly
Copy link
Author

jfly commented Mar 28, 2025

How could set_wakeup_fd know which signals you're interested in at all, and hence register a handler for them?

Wow, I'm sorry for the noise here. Of course it couldn't know that. Please ignore my comments above.

I'll send in a docs PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants