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

[Chris Angelico]

On Tue, Sep 25, 2018 at 1:10 AM Erik Bray  wrote:
>
> On Fri, Sep 21, 2018 at 12:58 AM Chris Angelico  wrote:
> >
> > On Fri, Sep 21, 2018 at 8:52 AM Kyle Lahnakoski  
> > wrote:
> > > 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.
> >
> > The time machine strikes again!
> >
> > https://docs.python.org/3/c-api/exceptions.html#signal-handling
>
> 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

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Fri, Sep 21, 2018 at 12:58 AM Chris Angelico  wrote:
>
> On Fri, Sep 21, 2018 at 8:52 AM Kyle Lahnakoski  
> wrote:
> > 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.
>
> The time machine strikes again!
>
> https://docs.python.org/3/c-api/exceptions.html#signal-handling

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Michael Selik]

On Thu, Sep 20, 2018, 3:52 PM Kyle Lahnakoski 
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.

>
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Chris Angelico]

On Fri, Sep 21, 2018 at 8:52 AM Kyle Lahnakoski  wrote:
> 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.

The time machine strikes again!

https://docs.python.org/3/c-api/exceptions.html#signal-handling

ChrisA
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Kyle Lahnakoski]

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.
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Greg Ewing]

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Greg Ewing]

Nathaniel Smith wrote:

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.


That should be easy enough, shouldn't it? When entering a new
frame, copy the mask state from the calling frame.


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.


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.


This would make me nervous, because context managers are used for all
kinds of things, and only some of them involve delicate resource
manipulation.


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

___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nathaniel Smith]

On Wed, Jun 28, 2017 at 6:19 AM, Greg Ewing  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 

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

[Greg Ewing]

Erik Bray wrote:

My question would be to
make that a language-level requirement of the context manager
protocol, or just something CPython does...


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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Wed, Jun 28, 2017 at 3:19 PM, Greg Ewing  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 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.

>> 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.

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...

> 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.

Exactly.

> 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.

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Wed, Jun 28, 2017 at 3:09 PM, Erik Bray  wrote:
> On Wed, Jun 28, 2017 at 2:26 PM, Nick Coghlan  wrote:
>> On 28 June 2017 at 21:40, Erik Bray  wrote:
>>> 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()
>>
>> 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
>
> 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.

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Greg Ewing]

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 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.

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Wed, Jun 28, 2017 at 2:26 PM, Nick Coghlan  wrote:
> On 28 June 2017 at 21:40, Erik Bray  wrote:
>> 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()
>
> 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

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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nick Coghlan]

On 28 June 2017 at 21:40, Erik Bray  wrote:
> 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()

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   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

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.



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?
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/