Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3.12.0a7 can fail to catch exceptions when an iterator is involved #103488

Closed
mattclay opened this issue Apr 12, 2023 · 14 comments
Closed
Assignees
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error

Comments

@mattclay
Copy link

mattclay commented Apr 12, 2023

Bug report

Python 3.12.0a7 (at least as of commit d65ed69) can fail to catch exceptions when an iterator is involved. The following code works on Python 3.11 but the ValueError is uncaught on Python 3.12:

#!/usr/bin/env python

def do_work():
    yield
    raise ValueError()


def main():
    try:
        for _ in do_work():
            if True is False:
                return
    except ValueError:
        pass


if __name__ == '__main__':
    main()

With this code in repro.py the following traceback is shown on Python 3.12:

Traceback (most recent call last):
  File "//repro.py", line 18, in <module>
    main()
  File "//repro.py", line 12, in main
    return
  File "//repro.py", line 5, in do_work
    raise ValueError()
ValueError

No error or output occurs under Python 3.11.

Your environment

Tested on Ubuntu 20.04.5 using the deadsnakes nightly build: python3.12 - 3.12.0~a7-98-gd65ed693a8-1+focal1

Linked PRs

@mattclay mattclay added the type-bug An unexpected behavior, bug, or error label Apr 12, 2023
@iritkatriel iritkatriel added release-blocker 3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Apr 12, 2023
@terryjreedy
Copy link
Member

terryjreedy commented Apr 12, 2023

Verified on latest 3.11/2 releases on Windows.
Edit revised: same if yield something but error in yield expression or before yield is caught. IE, Error must be after yield statement.

@pochmann
Copy link
Contributor

pochmann commented Apr 12, 2023

Line 12 being in the traceback instead of line 10 also seems wrong.

@terryjreedy
Copy link
Member

terryjreedy commented Apr 12, 2023

If return is changed to pass or continue, the exception is caught when the loop asks for the next value and gets an exception when executing the code after yield. I am slightly surprised that breaking (also not caught) or returning does not just gc the generator without executing the remaining (buggy) code. (Revised twice after more experiments.)

@terryjreedy
Copy link
Member

@mattclay Inserting 1/0 or better, an unbound name such as z is an easier way to generate an exception and easier to move or delete. 'NameError' should be the shortest exception name.

@gvanrossum
Copy link
Member

Can you post the reduced reproducer? And maybe the code object disassembly (python -m dis <file>)? It smells like a failure to generate or interpret the exception table correctly. Disassembly on the original reproducer shows odd gaps in the exception table that aren't there in 3.11.

@terryjreedy
Copy link
Member

@gvanrossum Who is 'who'? I reduced the raise and catch a bit but also added 3 debug prints to trace execution.

@mattclay
Copy link
Author

mattclay commented Apr 12, 2023

I simplified the reproducer a bit using the suggestion from @terryjreedy:

#!/usr/bin/env python

def do_work():
    yield
    x


def main():
    try:
        for _ in do_work():
            if True is False:
                return
    except NameError:
        pass


if __name__ == '__main__':
    main()

I've also attached the code object disassembly from Python 3.11 and 3.12 for the revised reproducer.

dis311.txt
dis312.txt (3.12.0a6)
dis312a7.txt (3.12.0a7, commit d65ed69)

@carljm
Copy link
Member

carljm commented Apr 12, 2023

It seems like this symptom could also result from a change in the timing of finalization of the do_work() generator on the stack in main(). If the generator is not exhausted, then the code after the yield will only run when the generator object is finalized, IIRC.

@carljm
Copy link
Member

carljm commented Apr 12, 2023

The change in behavior bisects to 4a1c58d

cc @markshannon

@terryjreedy
Copy link
Member

The conditional statement, a 'complex*' condition evaluating to false, so that the body is not executed, and a break/return in the non-executed body, seem necessary to get the error. False, 0, and 1-1 are simple. Comparisons tested (is, ==, and > so far) are complex.

@mattclay
Copy link
Author

Here's an even shorter reproducer:

def do_work():
    yield
    x


try:
    for _ in do_work():
        if True is False:
            break
except NameError:
    pass

Here's the -m dis output for the smaller reproducer:

dis311-simple.txt (3.11.3)
dis312a7-simple.txt (3.12.0a7, commit d65ed69)

@carljm
Copy link
Member

carljm commented Apr 12, 2023

Note the bisect blame commit doesn't change the compiler, so it's not an issue in the bytecode (unless it was a latent issue from earlier that was then exposed by a runtime change), it's something in the runtime behavior of generators changed in that commit. I confirmed that the dis output is identical before and after the blame commit, but after it the reproducer fails and before it it's fine.

@carljm
Copy link
Member

carljm commented Apr 13, 2023

It does appear, though, that the issue is that we fall into a hole in the exception table; the instruction offset used when calling get_exception_handler in the main frame is 23, which is the RETURN_VALUE in a return-None exit, and is outside the try/except block. That instruction offset seems wrong for where we should be in that frame when we get the exception from do_work, still tracking down why it's wrong.

Repro code I'm using
def do_work():
    yield
    x


try:
    for _ in do_work():
        if True is False:
            break
except NameError:
    pass
Bytecode
  0           0 RESUME                   0

  1           2 LOAD_CONST               0 (<code object do_work at 0x7fd5813db740, file "<exec>", line 1>)
              4 MAKE_FUNCTION            0
              6 STORE_NAME               0 (do_work)

  6           8 NOP

  7          10 PUSH_NULL
             12 LOAD_NAME                0 (do_work)
             14 CALL                     0
             24 GET_ITER
        >>   26 FOR_ITER                 9 (to 48)
             30 STORE_NAME               1 (_)

  8          32 LOAD_CONST               1 (True)
             34 LOAD_CONST               2 (False)
             36 IS_OP                    0
             38 POP_JUMP_IF_TRUE         1 (to 42)
             40 JUMP_BACKWARD            8 (to 26)

  9     >>   42 POP_TOP
             44 LOAD_CONST               3 (None)
             46 RETURN_VALUE

  7     >>   48 END_FOR
             50 LOAD_CONST               3 (None)
             52 RETURN_VALUE
        >>   54 PUSH_EXC_INFO

 10          56 LOAD_NAME                2 (NameError)
             58 CHECK_EXC_MATCH
             60 POP_JUMP_IF_FALSE        4 (to 70)
             62 POP_TOP

 11          64 POP_EXCEPT
             66 LOAD_CONST               3 (None)
             68 RETURN_VALUE

 10     >>   70 RERAISE                  0
        >>   72 COPY                     3
             74 POP_EXCEPT
             76 RERAISE                  1
ExceptionTable:
  10 to 38 -> 54 [0]
  42 to 42 -> 54 [0]
  48 to 48 -> 54 [0]
  54 to 62 -> 72 [1] lasti
  70 to 70 -> 72 [1] lasti

Disassembly of <code object do_work at 0x7fd5813db740, file "<exec>", line 1>:
  1           0 RETURN_GENERATOR
              2 POP_TOP
              4 RESUME                   0

  2           6 LOAD_CONST               0 (None)
              8 YIELD_VALUE              1
             10 RESUME                   1
             12 POP_TOP

  3          14 LOAD_GLOBAL              0 (x)
             26 POP_TOP
             28 LOAD_CONST               0 (None)
             30 RETURN_VALUE
        >>   32 STOPITERATION_ERROR
             34 RERAISE                  1
ExceptionTable:
  4 to 30 -> 32 [0] lasti

@markshannon markshannon self-assigned this Apr 13, 2023
markshannon added a commit that referenced this issue Apr 13, 2023
* Use return-offset, not yield-offset, so that instruction pointer is correct when sending to a generator or coroutine.
carljm added a commit to carljm/cpython that referenced this issue Apr 13, 2023
* main:
  pythongh-103479: [Enum] require __new__ to be considered a data type (pythonGH-103495)
  pythongh-103365: [Enum] STRICT boundary corrections (pythonGH-103494)
  pythonGH-103488: Use return-offset, not yield-offset. (pythonGH-103502)
  pythongh-103088: Fix test_venv error message to avoid bytes/str warning (pythonGH-103500)
  pythonGH-103082: Turn on branch events for FOR_ITER instructions. (python#103507)
  pythongh-102978: Fix mock.patch function signatures for class and staticmethod decorators (python#103228)
  pythongh-103462: Ensure SelectorSocketTransport.writelines registers a writer when data is still pending (python#103463)
  pythongh-95299: Rework test_cppext.py to not invoke setup.py directly (python#103316)
@mattclay
Copy link
Author

I've verified the issue has been resolved. I tested with the deadsnakes nightly build (commit 1aa376f).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

7 participants