Issue1423

classification
Title: Circular imports cause RuntimeError: maximum recursion depth exceeded
Type: crash Severity: major
Components: Core Versions: Jython 2.7
Milestone:
process
Status: pending Resolution: fixed
Dependencies: Superseder:
Assigned To: jeff.allen Nosy List: Lonebuddha, fwierzbicki, jeff.allen, zyasoft
Priority: high Keywords:

Created on 2009-08-05.10:56:09 by Lonebuddha, last changed 2015-09-01.19:09:24 by jeff.allen.

Files
File name Uploaded Description Edit Remove
circular_imports.zip Lonebuddha, 2009-08-05.10:56:09 Circular imports use case
Messages
msg4989 (view) Author: Andrey Sukhanov (Lonebuddha) Date: 2009-08-05.10:56:09
In case of circular imports Python shows "ImportError: cannot import
name <>" error and a stack trace, but Jython fails with "RuntimeError:
maximum recursion depth exceeded".

Use case description.
1. I have a package.
2. There are two modules in the package.
3. First one does "from . import <second module>"
4. Second one does "from . import <first module>"

See an example in the attached file. Try to run "main.py".
msg8468 (view) Author: Jim Baker (zyasoft) Date: 2014-05-21.20:43:03
Target beta 4
msg10181 (view) Author: Jeff Allen (jeff.allen) Date: 2015-08-30.07:31:44
The root of the problem seems to be here:
https://hg.python.org/jython/file/tip/src/org/python/core/imp.java#l1140
I think the aim of the code is to introduce the names of the imported modules into the namespace of the importing module. This should (and does) happen in the Python code where the import statement occurs, when importFromAs returns, but the comment points us to bug #1900 about importing Python modules into Java code.

The other factor here is this call to imp.find_module:
https://hg.python.org/jython/file/tip/src/org/python/core/PyModule.java#l106
Here "find" includes "load" and it is this call which causes the repeated attempts to load the sub-modules in the OP's example.

Nearby, a call to loadFromClassLoader is disabled by a comment, as if it might have caused a similar problem. (Archaeology has shown this hack to be 15 years old.)

In the context it is used above, PyModule.impAttr is needed only to return modules that are already loaded, not to try loading them again. (The module exists in sys.modules, at this point, even though we haven't finished executing it.)

I don't understand the general contract of PyModule.impAttr. But I think the least intrusive fix is to make the call to imp.find_module conditional on failing to find the module already in sys.modules. However, my first attempt is failing ant importest.
msg10185 (view) Author: Jeff Allen (jeff.allen) Date: 2015-08-30.22:08:50
That fixed the stack overflow. The issue #1900 fix indeed ensures that ri gets an attribute a, as it should, but it seems to happen earlier than in CPython.

CPython is unable to import ri.a on this second call, because the previous import is unfinished, and ri does not have the attribute a until the first import has returned. In Jython, this is done within the import processing. However, it does not seem like a language violation for Jython to succeed, see:
https://mail.python.org/pipermail/python-dev/2013-April/125041.html
msg10186 (view) Author: Jeff Allen (jeff.allen) Date: 2015-08-31.19:45:11
Fixed, I claim, in https://hg.python.org/jython/rev/b2c98eeaa744 .
msg10187 (view) Author: Jim Baker (zyasoft) Date: 2015-09-01.04:40:32
Very nice. One minor cleanup issue: we should just eliminate the commented-out code at https://hg.python.org/jython/rev/b2c98eeaa744#l2.44
msg10193 (view) Author: Jeff Allen (jeff.allen) Date: 2015-09-01.19:09:24
I don't like to mess with what I don't understand. However, it is a bit late now to ask Jim (https://hg.python.org/jython/rev/ccffd423f4b4#l1.28) and Samuele (https://hg.python.org/jython/rev/7d2145df96de#l6.17) what they were thinking, and if it was vital, we'd have missed it by now :), so ok. Looking at that first link, though, makes me crave the simplicity of the original.
History
Date User Action Args
2015-09-01 19:09:24jeff.allensetmessages: + msg10193
2015-09-01 04:40:33zyasoftsetmessages: + msg10187
2015-08-31 19:45:12jeff.allensetstatus: open -> pending
resolution: accepted -> fixed
messages: + msg10186
2015-08-30 22:08:50jeff.allensetmessages: + msg10185
2015-08-30 07:31:46jeff.allensetmessages: + msg10181
2015-04-19 22:40:52jeff.allensetassignee: jeff.allen
nosy: + jeff.allen
2014-07-01 04:06:54zyasoftsetpriority: high
2014-05-21 20:43:03zyasoftsetnosy: + zyasoft
messages: + msg8468
2014-05-04 20:28:06zyasoftsettitle: Unexpected behaviour is case of circular imports -> Circular imports cause RuntimeError: maximum recursion depth exceeded
2014-05-04 20:27:11zyasoftsetresolution: remind -> accepted
2013-02-19 20:48:03fwierzbickisetresolution: remind
versions: + Jython 2.7, - 2.5.0
2009-08-06 14:59:00fwierzbickisetnosy: + fwierzbicki
2009-08-05 10:56:09Lonebuddhacreate