assertion failure in add_signal_handler with prompt-toolkit

[faassen]

I'm trying to make prompt-toolkit (which has asyncio support) work with trio through this library. I don't actually know what I'm doing. :)

I had a persistent issue due to an assertion in trio-asyncio

assert sig not in self._signal_handlers, \ "Signal %d is already being caught" % (sig,)

My hypothesis is that prompt-toolkit in async mode assumes it can call this multiple times, but that trio-asyncio assumes you can't. Which one is right I cannot judge. If it's prompt-toolkit, I can try to communicate that to them.

I have an evil workaround where I write this in my application:

del signal.SIGWINCH

Because I noticed it was going wrong when prompt-toolkit detected SIGWINCH exists. But of course that's not a proper solution (and I just now lack resize support).

[faassen]

Here's some code that reproduces the issue. You need prompt_toolkit installed of course. If you enable the signal hack the code appears to work.

import asyncio
import trio_asyncio
from trio_asyncio import trio2aio
from prompt_toolkit import print_formatted_text, prompt as _prompt
from prompt_toolkit.eventloop.defaults import use_asyncio_event_loop

# workaround as otherwise prompt toolkit wants to re-add signal handler
# which trio_asyncio doesn't like because of an assertion
# this means the application can't respond to resizes anymore...

# import signal
# del signal.SIGWINCH

@trio2aio
async def prompt(*args, **kw):
    asyncio.get_event_loop().remove_signal_handler(28)
    return await _prompt(*args, async_=True, **kw)

async def async_main():
    use_asyncio_event_loop()
    text = await prompt("Give me input please: ")
    print_formatted_text(f"Hello world, your input was {text}")

trio_asyncio.run(async_main)

[njsmith]

As a rule of thumb, if it works on asyncio, but doesn't work on trio-asyncio, then it's probably a bug in trio-asyncio. (No matter what asyncio is "supposed" to do.) So, this is probably a bug in trio-asyncio!

It looks like it should be pretty straightforward to fix... right now trio-asyncio's add_signal_handler does:

        assert sig not in self._signal_handlers, \
            "Signal %d is already being caught" % (sig,)
        self._orig_signals[sig] = signal.signal(sig, self._handle_sig)
        self._signal_handlers[sig] = h

I think it should work to replace those lines of code with:

        if sig not in self._signal_handlers:
            self._orig_signals[sig] = signal.signal(sig, self._handle_sig)
        self._signal_handlers[sig] = h

Any interest in putting together a PR?

(@smurfix Do we need to add a call to self._signal_handlers[sig].cancel() as well? AFAICT this shouldn't do anything, but remove_signal_handler does call it...)

[faassen]

I can take a look at it later, but I don't guarantee I won't get seriously lost. :)

[njsmith]

That we can help with! :-) Feel free to ask in the issue or here...

[ugrash2097]

Hi ! Have you got any success in running a prompt toolkit full screen app on top of trio ?? It's been two days I'm trying and I really don't understand asyncio enough to debug...

Thank you anyway for the snippet and the wonderful library