classification
Title: import in finally results in SystemError
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, eric.snow, ncoghlan, serhiy.storchaka, tcaswell
Priority: normal Keywords:

Created on 2017-08-27 02:38 by tcaswell, last changed 2017-08-29 13:18 by tcaswell. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3217 merged serhiy.storchaka, 2017-08-27 07:20
Messages (7)
msg300911 - (view) Author: Thomas Caswell (tcaswell) * Date: 2017-08-27 02:38
Code to reproduce:

def import_in_finally_fail():
    try:
        print('yo')
    finally:
        import asyncio.queues as aq


Results in:

In [68]: import_in_finally_fail()
yo
---------------------------------------------------------------------------
SystemError                               Traceback (most recent call last)
<ipython-input-68-4a6d2efcb22b> in <module>()
----> 1 import_in_finally_fail()

/tmp/py2661VBj in import_in_finally_fail()

SystemError: 'finally' pops bad exception


The exact import does matter, but it needs to be at least 2 modules deep and be aliased.  

By patching cpython to put what finally pops off the stack in the error message it looks like it is the parent module of the import.

This was found via the Matplotlib test suite (ex https://travis-ci.org/matplotlib/matplotlib/jobs/262907186 ) and reported to pytest (https://github.com/pytest-dev/pytest/issues/2674)
msg300913 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-27 05:00
Confirmed. The bug is 3.7 only.
msg300914 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-27 06:33
The regression was introduced in issue30024. It was missed a difference between LOAD_ATTR and IMPORT_FROM. LOAD_ATTR replaces the object with the value of the attribute (do not changing the stack pointer). IMPORT_FROM keeps the original module on the stack and pushes the value of the attribute increasing the stack pointer.

There are two ways of fixing this issue.

1. Add two instructions ROT_TWO and POP_TOP after every IMPORT_FROM in the bytecode generated from "import a.b.c as d".

2. Make IMPORT_FROM popping the original module from the stack. The explicit DUP_TOP instruction should be inserted before every IMPORT_FROM in the bytecode generated from "from a import b, c".
msg300915 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-27 07:21
PR 3217 implements the first way. It also includes some refactoring. Remove all pyc-files before testing.
msg300984 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-29 12:47
New changeset 265fcc5fc25d65afb33fda480c603f1e974e374e by Serhiy Storchaka in branch 'master':
bpo-31286, bpo-30024: Fixed stack usage in absolute imports with (#3217)
https://github.com/python/cpython/commit/265fcc5fc25d65afb33fda480c603f1e974e374e
msg300985 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-29 12:50
Thank you for your report Thomas!

Don't forget to remove pyc-files before re-running the Matplotlib test suite.
msg300988 - (view) Author: Thomas Caswell (tcaswell) * Date: 2017-08-29 13:18
Your welcome!

Matplotlib ended up just moving the import out of the finally block once we
understood the issue.

Tom

On Tue, Aug 29, 2017 at 8:50 AM Serhiy Storchaka <report@bugs.python.org>
wrote:

>
> Serhiy Storchaka added the comment:
>
> Thank you for your report Thomas!
>
> Don't forget to remove pyc-files before re-running the Matplotlib test
> suite.
>
> ----------
> resolution:  -> fixed
> stage: patch review -> resolved
> status: open -> closed
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue31286>
> _______________________________________
>
History
Date User Action Args
2017-08-29 13:18:53tcaswellsetmessages: + msg300988
2017-08-29 12:50:01serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg300985

stage: patch review -> resolved
2017-08-29 12:47:46serhiy.storchakasetmessages: + msg300984
2017-08-27 07:21:48serhiy.storchakasetmessages: + msg300915
stage: patch review
2017-08-27 07:20:19serhiy.storchakasetpull_requests: + pull_request3257
2017-08-27 06:33:45serhiy.storchakasetmessages: + msg300914
2017-08-27 05:00:13serhiy.storchakasetnosy: + brett.cannon, ncoghlan, eric.snow, serhiy.storchaka
messages: + msg300913
2017-08-27 02:38:08tcaswellcreate