Author njs
Recipients Mark.Shannon, deleted0524, ncoghlan, njs, yselivanov
Date 2017-05-24.22:16:52
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1495664212.53.0.25102511019.issue29988@psf.upfronthosting.co.za>
In-reply-to
Content
> If all you need is that with foo: pass guarantees that either both or neither of __enter__ and __exit__ are called, for C context managers, and only C context managers, then the fix is trivial.

It would be nice to have it for 'async with foo: pass' as well, which is a little less trivial because the 'await' dance is inlined into the bytecode (see the initial post in this bug), but basically yes.

> Do you have any way to reliably test for this failure mode?

Unfortunately no, I haven't implemented one. Let's see, though...

The test I wrote for issue30039 demonstrates one way to trigger a signal on a precise bytecode, by writing some code in C that calls raise(SIGNALNUMBER), and then calling it immediately before the bytecode where we want the signal to be raised (simulating the situation where the signal happens to arrive while the C function is running -- note that raise() is convenient because unlike kill() it works cross-platform, even on Windows).

This might be sufficient for testing the 'async with' version; it looks like an __await__ method or iterator implemented in C and calling raise() would deliver a signal at a point that should be protected but isn't.

The tricky case is plain 'with'. We can write something like:

with lock:
    raise_signal()

and this gives bytecode like:

  1           0 LOAD_NAME                0 (lock)
              2 SETUP_WITH              12 (to 16)
              4 POP_TOP

  2           6 LOAD_NAME                1 (raise_signal)
              8 CALL_FUNCTION            0
             10 POP_TOP
             12 POP_BLOCK
             14 LOAD_CONST               0 (None)
        >>   16 WITH_CLEANUP_START

So the problem is that at offset 8 is where we can run arbitrary code, but the race condition is if a signal arrives between offsets 12 and 16.

One possibility would be to set up a chain of Py_AddPendingCall handlers, something like:

int chain1(void* _) {
    Py_AddPendingCall(chain2, 0);
    return 0;
}
int chain2(void* _) {
    Py_AddPendingCall(chain3, 0);
    return 0;
}
int chain3(void* _) {
    raise(SIGINT);
    return 0;
}

(or to reduce brittleness, maybe use the void* to hold an int controlling the length of the chain, which would make it easy to run tests with chains of length 1, 2, 3, ...)

......except consulting ceval.c I see that currently this won't work, because it looks like if you call Py_AddPendingCall from inside a pending call callback, then Py_MakePendingCalls will execute the newly added callback immediately after the first one returns. It could be made to work by having Py_MakePendingCalls do a first pass to check the current length of the queue, and then use that as the bound on how many calls it makes.
History
Date User Action Args
2017-05-24 22:16:52njssetrecipients: + njs, ncoghlan, Mark.Shannon, yselivanov, deleted0524
2017-05-24 22:16:52njssetmessageid: <1495664212.53.0.25102511019.issue29988@psf.upfronthosting.co.za>
2017-05-24 22:16:52njslinkissue29988 messages
2017-05-24 22:16:52njscreate