cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Replace _PyFrame_OpAlreadyRan by a check for incomplete frame

Open iritkatriel opened this issue 2 years ago • 6 comments

The _PyFrame_OpAlreadyRan function in frameobject.c is not covered by any tests.

For details see https://github.com/faster-cpython/ideas/issues/623.

Linked PRs

  • gh-119234

iritkatriel avatar Sep 09 '23 07:09 iritkatriel

Here's a demonstration of the (unlikely?) case for which _PyFrame_OpAlreadyRan() was added:

def spam(a, b, c):
    def eggs():
        return a, b, c
    return eggs

args = (1, 2, 3)
eggs1 = spam(*args)
print(eggs1.__closure__)
assert all(v is c.cell_contents for v, c in zip(args, eggs1.__closure__))

args = eggs1.__closure__
eggs2 = spam(*args)
print(eggs2.__closure__)
assert all(c not in eggs2.__closure__ for c in args)
assert all(v is c.cell_contents for v, c in zip(args, eggs2.__closure__))

However, this only needs _PyFrame_OpAlreadyRan() if it's possible to interrupt the eval loop before all MAKE_CELL instructions have executed. Per https://github.com/faster-cpython/ideas/issues/623#issuecomment-1715658784, it sounds like such a case is not possible. Thus, we might not have any way to add a test where _PyFrame_OpAlreadyRan() ever returns false. (We may end up removing _PyFrame_OpAlreadyRan() instead.)

@markshannon, could you verify that the eval loop can't be interrupted before all MAKE_CELL instructions have executed?

ericsnowcurrently avatar Sep 13 '23 16:09 ericsnowcurrently

Regardless, it would make sense to add a test that does something like the code I posted above.

ericsnowcurrently avatar Sep 13 '23 16:09 ericsnowcurrently

There could perhaps also be a pure C example.

iritkatriel avatar Sep 13 '23 16:09 iritkatriel

Yeah, and a pure-python test that uses a tracing hook to interrupt the code as early as possible. I suppose the same is true for a test that triggers the eval breaker as early as possible.

ericsnowcurrently avatar Sep 13 '23 16:09 ericsnowcurrently

Changed the title to reflect the decision in the discussion beginning with https://github.com/faster-cpython/ideas/issues/623#issuecomment-1717915148.

iritkatriel avatar Sep 15 '23 22:09 iritkatriel

@markshannon points out that this frame cannot be incomplete because it comes from a PyFrameObject (in PyFrame_GetVar). So we will just replace the check by an assertion.

iritkatriel avatar May 20 '24 21:05 iritkatriel