[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith added the comment:

This was fixed in April.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

already fixed, I just manually returned NULL. :)

I suppose we could change PyErr_NoMemory's definition in 3.3 to return a "void 
*" instead of "PyObject *" but I'd rather not.  In this case the warning caused 
me to examine the code and determine if it was in fact intended to do the right 
Python exception raising thing when NULL was returned from this non Python C 
API function.  In this case it was, but not all code can assume that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Jesús Cea Avión]

Jesús Cea Avión  added the comment:

Gregory patching was faster than my writing :-). Thanks for taking the effort 
:-).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Jesús Cea Avión]

Jesús Cea Avión  added the comment:

"PyErr_NoMemory()" returns always NULL, but it is declared as returning 
"PyObject *".

Could we change "return PyErr_NoMemory();" to "PyErr_NoMemory(); return 
NULL;"?. Maybe with some comment... A cast would silence the compiler but could 
be unclear. What do you think?

This is not a problem in 3.2 because the function patched returns a "PyObject 
*".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Roundup Robot]

Roundup Robot  added the comment:

New changeset b82a471d2c5e by Gregory P. Smith in branch '2.7':
Fix compiler warning related to issue #14331.  harmless.
http://hg.python.org/cpython/rev/b82a471d2c5e

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

That warning is correct, there's a bug in the code.  but given this is only a 
bug when PyMem_MALLOC returns NULL I do not expect this to be an issue for 
anyone who does not already have issues.

Regardless, I'm fixing it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Jesús Cea Avión]

Jesús Cea Avión  added the comment:

In fact, the compilation warning seems to expose a far more serious issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Jesús Cea Avión]

Jesús Cea Avión  added the comment:

Patch ad030571e6c0 introduces a compilation warning in Python 2.7. See


--
nosy: +jcea
versions: +Python 2.7, Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

2.7 and 3.2 have been fixed.  I'm keeping this open as a reminder to 
investigate how 3.3 behaves.  I'll fix it or close it after verifying that.

--
versions: +Python 3.3 -Python 2.7, Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Roundup Robot]

Roundup Robot  added the comment:

New changeset daed636a3536 by Gregory P. Smith in branch '3.2':
Fixes Issue #14331: Use significantly less stack space when importing modules by
http://hg.python.org/cpython/rev/daed636a3536

New changeset ad030571e6c0 by Gregory P. Smith in branch '2.7':
Fixes Issue #14331: Use significantly less stack space when importing modules by
http://hg.python.org/cpython/rev/ad030571e6c0

New changeset 5ad5625715b5 by Gregory P. Smith in branch 'default':
Empty merge; imports rewritten in 3.3. issue #14331 may no longer apply.
http://hg.python.org/cpython/rev/5ad5625715b5

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Changes by Gregory P. Smith :


Added file: 
http://bugs.python.org/file24930/malloc-import-pathbufs-py27.004.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

minor corresponding updated to the 2.7 patch as well - Patch 6 in 
reitveld/review.

The 3.2 patch from the previous comment is Patch 5 in reitveld/review.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

side by side code review of the 3.2 version revealed some missing PyMem_FREE 
calls.  patch updated.

--
Added file: 
http://bugs.python.org/file24929/malloc-import-pathbufs-py32.004.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

attaching the equivalent patch against python 3.2.  that could also use a pair 
of eyeballs for review.  it should show up as 'patch 4' in the rietveld reviews.

--
Added file: 
http://bugs.python.org/file24894/malloc-import-pathbufs-py32.003.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Updated to use PyErr_NoMemory().  Thanks Antoine!

I'm now working on this for 3.2 as well before I commit.

--
Added file: 
http://bugs.python.org/file24893/malloc-import-pathbufs-py27.003.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Thomas Wouters]

Thomas Wouters  added the comment:

For the record, as I said in the review of 002.diff: LGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Updated per review (style changes).

--
Added file: 
http://bugs.python.org/file24892/malloc-import-pathbufs-py27.002.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Brett Cannon]

Brett Cannon  added the comment:

Cursory glance has the patch LGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

It looks like MAXPATHLEN is 4096 on our systems.  The offending code that 
caused a stack overflow segfault shows over 100 Python/import.c function calls 
in its backtrace.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Here's a patch for python 2.7.  test cases pass but it could use review to see 
if I missed any free()s.

--
keywords: +patch
Added file: 
http://bugs.python.org/file24882/malloc-import-pathbufs-py27.001.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Eric V. Smith]

Changes by Eric V. Smith :


--
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Eric Snow]

Changes by Eric Snow :


--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14331] Python/import.c uses a lot of stack space due to MAXPATHLEN

[Gregory P. Smith]

New submission from Gregory P. Smith :

Python/import.c in 2.7 and 3.2 consume a lot of stack space when importing 
modules.  In stack constrained environments (think: 64k stack) this can cause a 
crash when you have a large chain of imports.

The bulk of this likely comes from places where a char buf[MAXPATHLEN+1] or 
similar is declared on the stack in the call chain.  MAXPATHLEN is commonly 
defined to be in the 256..1024 range depending on the OS.

Changing the code to not put the large buffer on the stack but to malloc and 
free it instead is a pretty straightforward re-factor.

import is being significantly reworked in 3.3 so I won't consider this issue 
there until after that settles as it may no longer apply.

--
assignee: gregory.p.smith
messages: 155981
nosy: brett.cannon, gregory.p.smith, ncoghlan, twouters
priority: normal
severity: normal
status: open
title: Python/import.c uses a lot of stack space due to MAXPATHLEN
versions: Python 2.7, Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com