New submission from Nathaniel Smith:
You might hope the interpreter would enforce the invariant that for 'with' and
'async with' blocks, either '__(a)enter__' and '__(a)exit__' are both called,
or else neither of them is called. But it turns out that this is not true once
KeyboardInterrupt gets involved – even if we write our (async) context manager
in C, or otherwise guarantee that the actual '__(a)enter__' and '__(a)exit__'
methods are immune to KeyboardInterrupt.
The invariant is *almost* preserved for 'with' blocks: there's one instruction
(SETUP_WITH) that atomically (wrt signals) calls '__enter__' and then enters
the implicit 'try' block, and there's one instruction (WITH_CLEANUP_START) that
atomically enters the implicit 'finally' block and then calls '__exit__'. But
there's a gap between exiting the 'try' block and WITH_CLEANUP_START where a
signal can arrive and cause us to exit without running the 'finally' block at
all. In this disassembly, the POP_BLOCK at offset 7 is the end of the 'try'
block; if a KeyboardInterrupt is raised between POP_BLOCK and
WITH_CLEANUP_START, then it will propagate out without '__exit__' being run:
In [2]: def f():
...: with a:
...: pass
...:
In [3]: dis.dis(f)
2 0 LOAD_GLOBAL 0 (a)
3 SETUP_WITH 5 (to 11)
6 POP_TOP
3 7 POP_BLOCK
8 LOAD_CONST 0 (None)
>> 11 WITH_CLEANUP_START
12 WITH_CLEANUP_FINISH
13 END_FINALLY
14 LOAD_CONST 0 (None)
17 RETURN_VALUE
For async context managers, the race condition is substantially worse, because
the 'await' dance is inlined into the bytecode:
In [4]: async def f():
...: async with a:
...: pass
...:
In [5]: dis.dis(f)
2 0 LOAD_GLOBAL 0 (a)
3 BEFORE_ASYNC_WITH
4 GET_AWAITABLE
5 LOAD_CONST 0 (None)
8 YIELD_FROM
9 SETUP_ASYNC_WITH 5 (to 17)
12 POP_TOP
3 13 POP_BLOCK
14 LOAD_CONST 0 (None)
>> 17 WITH_CLEANUP_START
18 GET_AWAITABLE
19 LOAD_CONST 0 (None)
22 YIELD_FROM
23 WITH_CLEANUP_FINISH
24 END_FINALLY
25 LOAD_CONST 0 (None)
28 RETURN_VALUE
Really the sequence from 3 BEFORE_ASYNC_WITH to 9 SETUP_ASYNC_WITH should be
atomic wrt signal delivery, and from 13 POP_BLOCK to 22 YIELD_FROM likewise.
This probably isn't the highest priority bug in practice, but I feel like it'd
be nice if this kind of basic language invariant could be 100% guaranteed, not
just 99% guaranteed :-). And the 'async with' race condition is plausible to
hit in practice, because if I have an '__aenter__' that's otherwise protected
from KeyboardInterrupt, then it can run for some time, and any control-C during
that time will get noticed just before the WITH_CLEANUP_START, so e.g. 'async
with lock: ...' might complete while still holding the lock.
The traditional solution would be to define single "super-instructions" that do
all of the work we want to be atomic. This would be pretty tricky here though,
because WITH_CLEANUP_START is a jump target (so naively we'd need to jump into
the "middle" of a hypothetical new super-instruction), and because the
implementation of YIELD_FROM kind of assumes that it's a standalone instruction
exposed directly in the bytecode. Probably there is some solution to these
issues but some cleverness would be required.
A alternative approach would be to keep the current bytecode, but somehow mark
certain stretches of bytecode as bad places to run signal handlers. The eval
loop's "check for signal handlers" code is run rarely, so we could afford to do
relatively expensive things like check a lookaside table that says "no signal
handlers when 13 < f_lasti <= 22". Or we could steal a bit in the opcode
encoding or something.
----------
components: Interpreter Core
messages: 291148
nosy: ncoghlan, njs, yselivanov
priority: normal
severity: normal
status: open
title: (async) with blocks and try/finally are not as KeyboardInterrupt-safe as
one might like
versions: Python 3.7
_______________________________________
Python tracker <[email protected]>
<http://bugs.python.org/issue29988>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
