[Python-ideas] Asynchronous exception handling around with/try statement borders

[Nathaniel Smith]

On Wed, Jun 28, 2017 at 6:19 AM, Greg Ewing <greg.ewing at canterbury.ac.nz> wrote:
> Erik Bray wrote:
>>
>> At this point a potentially
>> waiting SIGINT is handled, resulting in KeyboardInterrupt being raised
>> while inside the with statement's suite, and finally block, and hence
>> Lock.__exit__ are entered.
>
>
> Seems to me this is the behaviour you *want* in this case,
> otherwise the lock can be acquired and never released.
> It's disconcerting that it seems to be very difficult to
> get that behaviour with a pure Python implementation.

I agree :-)

>> I think it might be possible to
>> gain more consistency between these cases if pending signals are
>> checked/handled after any direct call to PyCFunction from within the
>> ceval loop.
>
>
> IMO that would be going in the wrong direction by making
> the C case just as broken as the Python case.
>
> Instead, I would ask what needs to be done to make this
> work correctly in the Python case as well as the C case.
>
> I don't think it's even possible to write Python code that
> does this correctly at the moment. What's needed is a
> way to temporarily mask delivery of asynchronous exceptions
> for a region of code, but unless I've missed something,
> no such facility is currently provided.

It's *almost* possible in some cases, by installing a specialized
signal handler which introspects the stack to see if we're in one of
these delicate regions of code. See:

    https://vorpus.org/blog/control-c-handling-in-python-and-trio/#how-trio-handles-control-c

The "almost" is because currently, I have a function decorator that
marks certain functions as needing protection against async signals,
which works by injecting a magic local variable into the function when
it starts. The problem is that this can't be done atomically, so if
you have an __exit__ method like:

    def __exit__(self, *args):
        # XX
        _this_function_is_protected = True
        self.release()

then it's possible for a signal to arrive at the point marked "XX",
and then your lock never gets released. One solution would be:
   https://bugs.python.org/issue12857
I've also been considering gross things like keeping a global WeakSet
of the code objects for all functions that have been registered for
async signal protection.

However, trio's solution looks a bit different than what you'd need
for a general python program, because the general strategy is that if
a signal arrives at a bad moment, we don't delay it (we can't!);
instead, we make a note to deliver it later. For an async framework
like trio this is fine, and in fact we need the "deliver the signal
later" facility anyway, because we need to handle the case where a
signal arrives while the event loop is polling for I/O and there isn't
any active task to deliver the signal to anyway. For a generic
solution in the interpreter, then I agree that it'd probably make more
sense to have a way to delay running the signal handler until an
appropriate moment.

> What would such a facility look like? One possibility
> would be to model it on the sigsetmask() system call, so
> there would be a function such as
>
>    mask_async_signals(bool)
>
> that turns delivery of async signals on or off.
>
> However, I don't think that would work. To fix the locking
> case, what we need to do is mask async signals during the
> locking operation, and only unmask them once the lock has
> been acquired. We might write a context manager with an
> __enter__ method like this:
>
>    def __enter__(self):
>       mask_async_signals(True)
>       try:
>          self.acquire()
>       finally:
>          mask_async_signals(False)
>
> But then we have the same problem again -- if a Keyboard
> Interrupt occurs after mask_async_signals(False) but
> before __enter__ returns, the lock won't get released.
>
> Another approach would be to provide a context manager
> such as
>
>    async_signals_masked(bool)
>
> Then the whole locking operation could be written as
>
>    with async_signals_masked(True):
>       lock.acquire()
>       try:
>          with async_signals_masked(False):
>             # do stuff here
>       finally:
>          lock.release()
>
> Now there's no possibility for a KeyboardInterrupt to
> be delivered until we're safely inside the body, but we've
> lost the ability to capture the pattern in the form of
> a context manager.

If async_signals_masked is implemented in C and can be used as a
decorator, then you could do:

class MyLock:
    @async_signals_masked(True)
    def __enter__(self):
        ...

    @async_signals_masked(True)
    def __exit__(self, *exc_info):
        ...

However, there's still a problem: in async code, you can yield out of
a async-signals-masked section, either because you have an async
context manager:

    @async_signals_masked(True)
    async def __aexit__(self, *exc_info):
        ...

or because you're using the context manager directly to protect some
delicate code:

    async def acquire(self):
        # Make sure KeyboardInterrupt can't leave us with a half-taken lock
        with async_signals_masked(True):
            if self._already_held:
                await self._lock_released_event
            ...

So what should this async_signals_masked state do when we yield out
from under it? If it's a thread-local, then the masking state will
"leak" into other async function callstacks (or similar for regular
generators), which is pretty bad. But it can't be just frame-local
data either, because we want the masking state to be inherited by and
subroutines we call from inside the masked block. This is why trio
uses the stack walking trick: it means that when you use 'yield' to
switch callstacks, the async signal masking state gets switched too,
automatically and atomically.

So maybe a better way would be to do something more like what trio
does. For example, we could have a flag on a function frame that says
"this frame (and the code in it) should not be interrupted", and then
in the bytecode loop when a signal arrives, walk up the call stack to
see if any of these flags are set before running the Python-level
signal handler. There's some efficiency and complexity questions here,
but maybe it's not too bad (signals are only received rarely, and
maybe there are some tricks to reduce the overhead).

> The only way out of this I can think of at the moment is
> to make the above pattern part of the context manager
> protocol itself. In other words, async exceptions are
> always masked while the __enter__ and __exit__ methods
> are executing, and unmasked while the body is executing.

This would make me nervous, because context managers are used for all
kinds of things, and only some of them involve delicate resource
manipulation. Masking async exceptions is a trade-off: if you do it at
the wrong place, then you can end up with a program that refuses to
respond to control-C, which is pretty frustrating. There are also some
rather nasty cases, like I think Popen.__exit__ might block waiting
for SIGCHLD to be delivered?

And anyway, you still have to solve the problem of how you communicate
this state to subroutines called by __(a)enter__ and __(a)exit__, but
not let it leak when you yield. Once you solve that I think you have
95% of the machinery you need to make this user-controllable.

-n

-- 
Nathaniel J. Smith -- https://vorpus.org