Asynchronous exception handling around with/try statement borders

Hi folks, I normally wouldn't bring something like this up here, except I think that there is possibility of something to be done--a language documentation clarification if nothing else, though possibly an actual code change as well. I've been having an argument with a colleague over the last couple days over the proper way order of statements when setting up a try/finally to perform cleanup of some action. On some level we're both being stubborn I think, and I'm not looking for resolution as to who's right/wrong or I wouldn't bring it to this list in the first place. The original argument was over setting and later restoring os.environ, but we ended up arguing over threading.Lock.acquire/release which I think is a more interesting example of the problem, and he did raise a good point that I do want to bring up. </prologue> My colleague's contention is that given lock = threading.Lock() this is simply *wrong*: lock.acquire() try: do_something() finally: lock.release() whereas this is okay: with lock: do_something() Ignoring other details of how threading.Lock is actually implemented, assuming that Lock.__enter__ calls acquire() and Lock.__exit__ calls release() then as far as I've known ever since Python 2.5 first came out these two examples are semantically *equivalent*, and I can't find any way of reading PEP 343 or the Python language reference that would suggest otherwise. However, there *is* a difference, and has to do with how signals are handled, particularly w.r.t. context managers implemented in C (hence we are talking CPython specifically): If Lock.__enter__ is a pure Python method (even if it maybe calls some C methods), and a SIGINT is handled during execution of that method, then in almost all cases a KeyboardInterrupt exception will be raised from within Lock.__enter__--this means the suite under the with: statement is never evaluated, and Lock.__exit__ is never called. You can be fairly sure the KeyboardInterrupt will be raised from somewhere within a pure Python Lock.__enter__ because there will usually be at least one remaining opcode to be evaluated, such as RETURN_VALUE. Because of how delayed execution of signal handlers is implemented in the pyeval main loop, this means the signal handler for SIGINT will be called *before* RETURN_VALUE, resulting in the KeyboardInterrupt exception being raised. Standard stuff. However, if Lock.__enter__ is a PyCFunction things are quite different. If you look at how the SETUP_WITH opcode is implemented, it first calls the __enter__ method with _PyObjet_CallNoArg. If this returns NULL (i.e. an exception occurred in __enter__) then "goto error" is executed and the exception is raised. However if it returns non-NULL the finally block is set up with PyFrame_BlockSetup and execution proceeds to the next opcode. 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. Long story short, because Lock.__enter__ is a C function, assuming that it succeeds normally then with lock: do_something() always guarantees that Lock.__exit__ will be called if a SIGINT was handled inside Lock.__enter__, whereas with lock.acquire() try: ... finally: lock.release() there is at last a small possibility that the SIGINT handler is called after the CALL_FUNCTION op but before the try/finally block is entered (e.g. before executing POP_TOP or SETUP_FINALLY). So the end result is that the lock is held and never released after the KeyboardInterrupt (whether or not it's handled somehow). Whereas, again, if Lock.__enter__ is a pure Python function there's less likely to be any difference (though I don't think the possibility can be ruled out entirely). At the very least I think this quirk of CPython should be mentioned somewhere (since in all other cases the semantic meaning of the "with:" statement is clear). However, 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. Sorry for the tl;dr; any thoughts?

On 28 June 2017 at 21:40, Erik Bray <erik.m.bray@gmail.com> wrote:
Technically both are slightly racy with respect to async signals (e.g. KeyboardInterrupt), but the with statement form is less exposed to the problem (since it does more of its work in single opcodes). Nathaniel Smith posted a good write-up of the technical details to the issue tracker based on his work with trio: https://bugs.python.org/issue29988 Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On Wed, Jun 28, 2017 at 2:26 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Interesting; thanks for pointing this out. Part of me felt like this has to have come up before but my searching didn't bring this up somehow (and even then it's only a couple months old itself). I didn't think about the possible race condition before WITH_CLEANUP_START, but obviously that's a possibility as well. Anyways since this is already acknowledged as a real bug I guess any further followup can happen on the issue tracker. Thanks, Erik

On Wed, Jun 28, 2017 at 3:09 PM, Erik Bray <erik.m.bray@gmail.com> wrote:
On second thought, maybe there is a case to made w.r.t. making a documentation change about the semantics of the `with` statement: The old-style syntax cannot make any guarantees about atomicity w.r.t. async events. That is, there's no way syntactically in Python to declare that no exception will be raised between "lock.acquire()" and the setup of the "try/finally" blocks. However, if issue-29988 were *fixed* somehow (and I'm not convinced it can't be fixed in the limited case of `with` statements) then there really would be a major semantic difference of the `with` statement in that it does support this invariant. Then the question is whether that difference be made a requirement of the language (probably too onerous a requirement?), or just a feature of CPython (which should still be documented one way or the other IMO). Erik

Erik Bray wrote:
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.
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. 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. 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. -- Greg

On Wed, Jun 28, 2017 at 3:19 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
I think normally you're right--this is the behavior you would *want*, but not the behavior that's consistent with how Python implements the `with` statement, all else being equal. Though it's still not entirely fair either because if Lock.__enter__ were pure Python somehow, it's possible the exception would be raised either before or after the lock is actually marked as "acquired", whereas in the C implementation acquisition of the lock will always succeed (assuming the lock was free, and no other exceptional conditions) before the signal handler is executed.
You have a point there, but at the same time the Python case, while "broken" insofar as it can lead to broken code, seems correct from the Pythonic perspective. The other possibility would be to actually change the semantics of the `with` statement. Or as you mention below, a way to temporarily mask signals...
Exactly.
I think so too. That's more or less in line with Nick's idea on njs's issue (https://bugs.python.org/issue29988) of an ATOMIC_UNTIL opcode. That's just one implementation possibility. My question would be to make that a language-level requirement of the context manager protocol, or just something CPython does... Thanks, Erik

Erik Bray wrote:
I think it should be a language-level requirement, otherwise it's not much use. Note that it's different from some existing CPython-only behaviour such as refcounting, because it's possible to code around those things on other implementations that don't provide the same guarantees, but here there's *no* way to code around it. At the very least, it should be a documented guarantee in CPython, not just something left "up to the implementation". -- Greg

On Wed, Jun 28, 2017 at 6:19 AM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
I agree :-)
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-hand... 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.
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).
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

Nathaniel Smith wrote:
That should be easy enough, shouldn't it? When entering a new frame, copy the mask state from the calling frame.
That would make it impossible to temporarily unmask async signals in a region where they're masked. An example of a situation where you might want to do that is in an implementation of lock.acquire(). If the thread needs to block while waiting for the lock to become available, you probably want to allow ctrl-C to interrupt the thread while it's blocked.
The next step I had in mind was to extend the context manager protocol so that the context manager can indicate whether it wants async signals masked, so it would only happen for things like lock that really need it. -- Greg

On Wed, Jun 28, 2017 at 9:14 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Right, that approach would be semantically equivalent to the walking-the-call-stack approach, just it shifts some of the cost around (it makes signals cheaper by making each function call a tiny bit more expensive).
So trio actually does allow this kind of nesting -- for any given frame the special flag can be unset, set to True, or set to False, and the signal handler walks up until it finds the first one frame where it's set and uses that. So I guess the equivalent would be two flags, or a little enum field in the frame object, or something like that.
A magic (implemented in C) decorator like @async_signals_masked I think would be the simplest way to do this extension. (Or maybe better call it @no_signal_delivery, because it would have to block all running of signal handlers; the interpreter doesn't know whether a signal handler will raise an exception until it calls it.) -n -- Nathaniel J. Smith -- https://vorpus.org

Nathaniel Smith wrote:
A magic (implemented in C) decorator like @async_signals_masked I think would be the simplest way to do this extension.
I don't have a good feeling about that approach. While implementing the decorator in C might be good enough in CPython to ensure no window of opportunity exists to leak a signal, that might not be true in other Python implementations. -- Greg

On 2017-06-28 07:40, Erik Bray wrote:
Hi folks,
Since the java.lang.Thread.stop() "debacle", it has been obvious that stopping code to run other code has been dangerous. KeyboardInterrupt (any interrupt really) is dangerous. Now, we can probably code a solution, but how about we remove the danger: I suggest we remove interrupts from Python, and make them act more like java.lang.Thread.interrupt(); setting a thread local bit to indicate an interrupt has occurred. Then we can write explicit code to check for that bit, and raise an exception in a safe place if we wish. This can be done with Python code, or convenient places in Python's C source itself. I imagine it would be easier to whitelist where interrupts can raise exceptions, rather than blacklisting where they should not. In the meantime, my solution is to spawn new threads to do the work, while the main thread has the sole purpose to sleep, and set the "please stop" flag upon interrupt.

On Fri, Sep 21, 2018 at 8:52 AM Kyle Lahnakoski <klahnakoski@mozilla.com> wrote:
The time machine strikes again! https://docs.python.org/3/c-api/exceptions.html#signal-handling ChrisA

On Fri, Sep 21, 2018 at 12:58 AM Chris Angelico <rosuav@gmail.com> wrote:
Although my original post did not explicitly mention PyErr_CheckSignals() and friends, it had already taken that into account and it is not a silver bullet, at least w.r.t. the exact issue I raised, which had to do with the behavior of context managers versus the setup() try: do_thing() finally: cleanup() pattern, and the question of how signals are handled between Python interpreter opcodes. There is a still-open bug on the issue tracker discussing the exact issue in greater details: https://bugs.python.org/issue29988

On Tue, Sep 25, 2018 at 1:10 AM Erik Bray <erik.m.bray@gmail.com> wrote:
To be fair, your post not only didn't mention CheckSignals, but it almost perfectly described its behaviour. So I stand by my response. :) I don't think the system needs to be replaced; it ought to be possible to resolve the context manager issue without tossing out the existing code. ChrisA

On Thu, Sep 20, 2018, 3:52 PM Kyle Lahnakoski <klahnakoski@mozilla.com> wrote:
KeyboardInterrupt (any interrupt really) is dangerous. Now, we can probably code a solution, but how about we remove the danger
The other day I accidentally fork-bombed myself with Python os.fork in an infinite loop. Whoops. It seems to me that Python's design philosophy is to make the safe things beautiful and efficient, but not to remove the dangerous things. I'd be supportive of a proposal that makes threading safer without removing capabilities for those that want them.

On 28 June 2017 at 21:40, Erik Bray <erik.m.bray@gmail.com> wrote:
Technically both are slightly racy with respect to async signals (e.g. KeyboardInterrupt), but the with statement form is less exposed to the problem (since it does more of its work in single opcodes). Nathaniel Smith posted a good write-up of the technical details to the issue tracker based on his work with trio: https://bugs.python.org/issue29988 Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia

On Wed, Jun 28, 2017 at 2:26 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Interesting; thanks for pointing this out. Part of me felt like this has to have come up before but my searching didn't bring this up somehow (and even then it's only a couple months old itself). I didn't think about the possible race condition before WITH_CLEANUP_START, but obviously that's a possibility as well. Anyways since this is already acknowledged as a real bug I guess any further followup can happen on the issue tracker. Thanks, Erik

On Wed, Jun 28, 2017 at 3:09 PM, Erik Bray <erik.m.bray@gmail.com> wrote:
On second thought, maybe there is a case to made w.r.t. making a documentation change about the semantics of the `with` statement: The old-style syntax cannot make any guarantees about atomicity w.r.t. async events. That is, there's no way syntactically in Python to declare that no exception will be raised between "lock.acquire()" and the setup of the "try/finally" blocks. However, if issue-29988 were *fixed* somehow (and I'm not convinced it can't be fixed in the limited case of `with` statements) then there really would be a major semantic difference of the `with` statement in that it does support this invariant. Then the question is whether that difference be made a requirement of the language (probably too onerous a requirement?), or just a feature of CPython (which should still be documented one way or the other IMO). Erik

Erik Bray wrote:
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.
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. 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. 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. -- Greg

On Wed, Jun 28, 2017 at 3:19 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
I think normally you're right--this is the behavior you would *want*, but not the behavior that's consistent with how Python implements the `with` statement, all else being equal. Though it's still not entirely fair either because if Lock.__enter__ were pure Python somehow, it's possible the exception would be raised either before or after the lock is actually marked as "acquired", whereas in the C implementation acquisition of the lock will always succeed (assuming the lock was free, and no other exceptional conditions) before the signal handler is executed.
You have a point there, but at the same time the Python case, while "broken" insofar as it can lead to broken code, seems correct from the Pythonic perspective. The other possibility would be to actually change the semantics of the `with` statement. Or as you mention below, a way to temporarily mask signals...
Exactly.
I think so too. That's more or less in line with Nick's idea on njs's issue (https://bugs.python.org/issue29988) of an ATOMIC_UNTIL opcode. That's just one implementation possibility. My question would be to make that a language-level requirement of the context manager protocol, or just something CPython does... Thanks, Erik

Erik Bray wrote:
I think it should be a language-level requirement, otherwise it's not much use. Note that it's different from some existing CPython-only behaviour such as refcounting, because it's possible to code around those things on other implementations that don't provide the same guarantees, but here there's *no* way to code around it. At the very least, it should be a documented guarantee in CPython, not just something left "up to the implementation". -- Greg

On Wed, Jun 28, 2017 at 6:19 AM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
I agree :-)
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-hand... 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.
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).
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

Nathaniel Smith wrote:
That should be easy enough, shouldn't it? When entering a new frame, copy the mask state from the calling frame.
That would make it impossible to temporarily unmask async signals in a region where they're masked. An example of a situation where you might want to do that is in an implementation of lock.acquire(). If the thread needs to block while waiting for the lock to become available, you probably want to allow ctrl-C to interrupt the thread while it's blocked.
The next step I had in mind was to extend the context manager protocol so that the context manager can indicate whether it wants async signals masked, so it would only happen for things like lock that really need it. -- Greg

On Wed, Jun 28, 2017 at 9:14 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Right, that approach would be semantically equivalent to the walking-the-call-stack approach, just it shifts some of the cost around (it makes signals cheaper by making each function call a tiny bit more expensive).
So trio actually does allow this kind of nesting -- for any given frame the special flag can be unset, set to True, or set to False, and the signal handler walks up until it finds the first one frame where it's set and uses that. So I guess the equivalent would be two flags, or a little enum field in the frame object, or something like that.
A magic (implemented in C) decorator like @async_signals_masked I think would be the simplest way to do this extension. (Or maybe better call it @no_signal_delivery, because it would have to block all running of signal handlers; the interpreter doesn't know whether a signal handler will raise an exception until it calls it.) -n -- Nathaniel J. Smith -- https://vorpus.org

Nathaniel Smith wrote:
A magic (implemented in C) decorator like @async_signals_masked I think would be the simplest way to do this extension.
I don't have a good feeling about that approach. While implementing the decorator in C might be good enough in CPython to ensure no window of opportunity exists to leak a signal, that might not be true in other Python implementations. -- Greg

On 2017-06-28 07:40, Erik Bray wrote:
Hi folks,
Since the java.lang.Thread.stop() "debacle", it has been obvious that stopping code to run other code has been dangerous. KeyboardInterrupt (any interrupt really) is dangerous. Now, we can probably code a solution, but how about we remove the danger: I suggest we remove interrupts from Python, and make them act more like java.lang.Thread.interrupt(); setting a thread local bit to indicate an interrupt has occurred. Then we can write explicit code to check for that bit, and raise an exception in a safe place if we wish. This can be done with Python code, or convenient places in Python's C source itself. I imagine it would be easier to whitelist where interrupts can raise exceptions, rather than blacklisting where they should not. In the meantime, my solution is to spawn new threads to do the work, while the main thread has the sole purpose to sleep, and set the "please stop" flag upon interrupt.

On Fri, Sep 21, 2018 at 8:52 AM Kyle Lahnakoski <klahnakoski@mozilla.com> wrote:
The time machine strikes again! https://docs.python.org/3/c-api/exceptions.html#signal-handling ChrisA

On Fri, Sep 21, 2018 at 12:58 AM Chris Angelico <rosuav@gmail.com> wrote:
Although my original post did not explicitly mention PyErr_CheckSignals() and friends, it had already taken that into account and it is not a silver bullet, at least w.r.t. the exact issue I raised, which had to do with the behavior of context managers versus the setup() try: do_thing() finally: cleanup() pattern, and the question of how signals are handled between Python interpreter opcodes. There is a still-open bug on the issue tracker discussing the exact issue in greater details: https://bugs.python.org/issue29988

On Tue, Sep 25, 2018 at 1:10 AM Erik Bray <erik.m.bray@gmail.com> wrote:
To be fair, your post not only didn't mention CheckSignals, but it almost perfectly described its behaviour. So I stand by my response. :) I don't think the system needs to be replaced; it ought to be possible to resolve the context manager issue without tossing out the existing code. ChrisA

On Thu, Sep 20, 2018, 3:52 PM Kyle Lahnakoski <klahnakoski@mozilla.com> wrote:
KeyboardInterrupt (any interrupt really) is dangerous. Now, we can probably code a solution, but how about we remove the danger
The other day I accidentally fork-bombed myself with Python os.fork in an infinite loop. Whoops. It seems to me that Python's design philosophy is to make the safe things beautiful and efficient, but not to remove the dangerous things. I'd be supportive of a proposal that makes threading safer without removing capabilities for those that want them.
participants (7)
-
Chris Angelico
-
Erik Bray
-
Greg Ewing
-
Kyle Lahnakoski
-
Michael Selik
-
Nathaniel Smith
-
Nick Coghlan