python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
6.19k stars 341 forks source link

Document how Trio handles signals #673

Open Nikratio opened 6 years ago

Nikratio commented 6 years ago

The documentation and discussion in issue #550 leaves me with the impression that trio is installing some signal handlers. This concerns me, because I would like to use trio in an application that does its own Ctrl-C handling at the C level. Unfortunately I could not find any specifics about what trio is doing (and potential ways to disable it) in the documentation.

It would be great if someone could clarify :-).

njsmith commented 6 years ago

By default, Trio does install a handler for SIGINT (control-C) when you entering trio.run and then uninstalls it when trio.run finishes. This only happens if:

And, if you overwrite Trio's signal handler witha different one, then that's fine, nothing breaks. The uninstall code will even detect this case, and leave your signal handler in place rather than restoring the original signal handler.

The only other time Trio installs any signal handlers is if you explicitly use open_signal_receiver.

So... I don't think you have anything to worry about. Trio's control-C handling acts exactly like the interpreter's default handling, and if whatever you're doing would work on the vanilla interpreter then it should work identically when trio gets involved.

njsmith commented 6 years ago

....ugh I just looked at the interpreter source code and I think i might be wrong here actually, basically because of an interpreter bug.

What I wrote above is what the trio code tries to do. But, when it checks whether the current SIGINT handler is the default one python installed, it does that using the signal.getsignal function. And looking at the source, getsignal returns the last signal handler that was set from python. If some C code has, in the mean time, set its own signal handler, then getsignal returns stale/incorrect information. Result: it's possible to get intoa state where trio thinks it's just replacing the normal python SIGINT handler, but actually it's replacing your custom C SIGINT handler.

There is a gross workaround available: between starting the python interpreter and installing your C level handler, use signal.signal to install a custom python level SIGINT handler. Then trio will see that something funny is going on and disable it's normal behavior.

The ideal real fix would be for the python signal module to stop assuming that all signal calls will be made through it. But that's a non-trivial change and can't happen before 3.8 at the earliest, so we should open an issue at bugs.python.org but also think if there's a better possible workaround.

Ideally trio could detect automatically when this has happened. The sigaction function does let you query for the current C level signal handler without changing it. But... even if we figure out how to call it using ctypes or something, there may not be any way to figure out what python thinks the C level handler should be, to compare it to.

Alternatively, yeah, we could provide some kwarg to run. It makes me grit my teetha little because it's a (hopefully) temporary workaroind for a confusing and obscure issue, but if it's causing real problems and there aren't any better options then we might have to anyway.

Nikratio commented 6 years ago

Wow, thanks for checking this in the source as well! Installing a fake handler with the signal module before installing the real one in C will work fine for me as a workaround. Thank a lot for the quick help!

njsmith commented 6 years ago

Looking more closely at PyInit__signal, it looks like everything would also work if you set up your SIGINT handler before you initialize the Python interpreter – in this case Python will skip installing its normal SIGINT handler, and trio will notice this and skip installing its normal SIGINT handler.

njsmith commented 6 years ago

It looks like the upstream bug here is bpo-13285, and here's my post about this there: https://bugs.python.org/issue13285#msg325700

mikaelho commented 6 years ago

Hello. Would the issue described below be related, or should I create a separate issue?

I am trying to use trio in Pythonista, an iOS app running Python 3.6.1. I am able to use asyncio and aiohttp, but getting quite frustrated with the cancel semantics.

Trying to run the following simple example:

import trio

async def async_double(x):
    return 2 * x

trio.run(async_double, 3)  # returns 6

I get this trace:

Traceback (most recent call last):
  File "/private/var/mobile/Containers/Shared/AppGroup/DB278AED-4773-408E-8500-E06A4DF50B70/Pythonista3/Documents/Testailut/triobasic.py", line 6, in <module>
    trio.run(async_double, 3)  # returns 6
  File "/private/var/mobile/Containers/Shared/AppGroup/DB278AED-4773-408E-8500-E06A4DF50B70/Pythonista3/Documents/site-packages-3/trio/_core/_run.py", line 1308, in run
    runner.deliver_ki, restrict_keyboard_interrupt_to_checkpoints
  File "/var/containers/Bundle/Application/29748688-5C34-4B85-A54D-9874F692C45A/Pythonista3.app/Frameworks/Py3Kit.framework/pylib/contextlib.py", line 83, in __enter__
    return next(self.gen)
  File "/private/var/mobile/Containers/Shared/AppGroup/DB278AED-4773-408E-8500-E06A4DF50B70/Pythonista3/Documents/site-packages-3/trio/_core/_ki.py", line 179, in ki_manager
    not is_main_thread()
  File "/private/var/mobile/Containers/Shared/AppGroup/DB278AED-4773-408E-8500-E06A4DF50B70/Pythonista3/Documents/site-packages-3/trio/_util.py", line 100, in is_main_thread
    signal.signal(signal.SIGINT, signal.getsignal(signal.SIGINT))
  File "/var/containers/Bundle/Application/29748688-5C34-4B85-A54D-9874F692C45A/Pythonista3.app/Frameworks/Py3Kit.framework/pylib/signal.py", line 48, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object

This is surely partially due to Pythonista implementation, but seems related tonthis issue. Are there any non-C workarounds? Thanks!

njsmith commented 6 years ago

@mikaelho How odd... what does signal.getsignal(signal.SIGINT) return in Pythonista?

That is probably a somewhat different issue, but there is a connection to this issue: that signal.signal(signal.SIGINT, signal.getsignal(signal.SIGINT)) dance that Trio does there is supposed to be a no-op, but in @Nikratio's case it actually could break things :-(.

@Nikratio have you confirmed that setting a fake signal handler actually works? Now that I look at Trio's is_main_thread I am dubious :-(

njsmith commented 6 years ago

CC @Fuyukai, since one option we may have to consider here is reverting #462

mikaelho commented 6 years ago

@njsmith, signal.getsignal(signal.SIGINT) returns None on Pythonista.

njsmith commented 6 years ago

Let's move the discussion of Pythonista over to #684.

Nikratio commented 6 years ago

@njsmith Nope, I did not check whether it works. In the meantime, increased trio prevalence throughout the code meant that the custom signal handlers was no longer required.

graingert commented 4 years ago

@njsmith how about signal.set_wakeup_fd(-2)?

def is_main_thread():
    try:
        signal.set_wakeup_fd(-2)
    except OSError:
        return True
    except ValueError:
        return False
    sys.exit("OH NO WE SET THE WAKEUP FD TO -2")
njsmith commented 4 years ago

@graingert Interesting idea!

On Unix, file descriptors are signed ints, and -2 is never a valid file descriptor. So this definitely works reliably there.

On Windows, can -2 ever be a valid value? This gets obscure quickly.

According to https://docs.microsoft.com/en-us/windows/win32/winsock/socket-data-type-2, in theory a real socket handle could have the value -2:

Windows Sockets handles have no restrictions, other than that the value INVALID_SOCKET is not a valid socket. Socket handles may take any value in the range 0 to INVALID_SOCKET–1.

(Socket handles are unsigned, so -1 is the same as INVALID_SOCKET, which is the same as the maximum possible socket value.)

Could this ever happen in practice? On the vast majority of current systems, socket handles are kernel handles, and kernel handles are guaranteed to be a multiple of 4, so -2 can't happen. The only time socket handles aren't kernel handles are if you have some wacky "Layered Service Provider" installed. And even then I assume it's rare that a LSP will happen to pick -2 as a socket handle. (There's a lot more discussion of LSPs in #52.)

So... in principle it's possible for set_wakeup_fd(-2) to succeed, but it will be extraordinarily uncommon in practice.

I'm not quite sure what to do with that conclusion. All of the issues we're talking about here are very edge-case-y: the original code ran into a bug with old versions of gevent, which has since been fixed. The new code has a problem with C libraries that set their own SIGINT handlers, but we don't have any users actually complaining about this as a practical issue currently. The set_wakeup_fd(-2) trick is theoretically not guaranteed to work, but actually will in almost all cases. So the question is which of these edge-cases is the least bad.

njsmith commented 4 years ago

It would probably also be possible to remove all instances of is_main_thread from Trio, by instead just trying to perform the operations and then handling failures.

Also in practice, this is currently having exactly zero effect on any users, so the best answer may be to defer any changes it until it causes an actual problem.

Nikratio commented 3 years ago

I've come back to this issue because Python signal handling in general is a bit of a pain.

Having read through all of the above, it seems to me that in order to use my own, C-level signal handlers the right thing to do is still:

  1. Set up my own signal handlers in C
  2. Set up Python signal handler for these signals (which won't actually be executed since the Python C handlers are not called, but will be noticed by trio)
  3. Start using Trio

Is that correct? If so, can we maybe add this to the Trio documentation and close this bug? I think a new (and very short) "Signal handling" section under "Trio's Core Functionality" would be the best place.

(Hopefully this place could also describe what happens if no custom signal handling is done at all, and the user presses Ctrl-C. Will all tasks receive a KeyboardInterrupt? Or will they just stop executing? Or will they keep running?)