Repository navigation
Undetected error in exception handling #46800
Description
| BPO | 2548 |
|---|---|
| Nosy | @warsaw, @brettcannon, @theller, @amauryfa, @pitrou, @devdanzin, @benjaminp |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2010-08-16.21:38:15.544>
created_at = <Date 2008-04-04.08:16:41.003>
labels = ['interpreter-core', 'type-crash']
title = 'Undetected error in exception handling'
updated_at = <Date 2010-08-16.21:38:15.543>
user = 'https://github.com/theller'bugs.python.org fields:
activity = <Date 2010-08-16.21:38:15.543>
actor = 'BreamoreBoy'
assignee = 'none'
closed = True
closed_date = <Date 2010-08-16.21:38:15.544>
closer = 'BreamoreBoy'
components = ['Interpreter Core']
creation = <Date 2008-04-04.08:16:41.003>
creator = 'theller'
dependencies = []
files = ['11314']
hgrepos = []
issue_num = 2548
keywords = ['patch', 'needs review']
message_count = 18.0
messages = ['64920', '64954', '64961', '64983', '65053', '66398', '68018', '70482', '71095', '71107', '71651', '72163', '72176', '72179', '72181', '101353', '110185', '110215']
nosy_count = 10.0
nosy_names = ['barry', 'nnorwitz', 'brett.cannon', 'theller', 'jafo', 'amaury.forgeotdarc', 'pitrou', 'ajaksu2', 'benjamin.peterson', 'BreamoreBoy']
pr_nums = []
priority = 'critical'
resolution = 'wont fix'
stage = None
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue2548'
versions = ['Python 2.6']
Activity
theller commentedon Apr 4, 2008
[Found by Daniel Diniz (ajaksu2), see issue bpo-2542]
The following code triggers an undetected error with a debug build:
"""
import sys
def g():
try:
return g()
except:
return sys.exc_info()
g()
print 42
"""
Running the code prints this:
C:\svn\trunk\PCbuild>
amauryfa commentedon Apr 5, 2008
Analysis: the primary recursion error is correctly raised, but then
there is a call to PyErr_NormalizeException, which calls
PyEval_CallObject, which increases the stack depth and hit the recursion
limit again...
python2.5 don't have the problem because PyEval_CallObject did not check
the recursion limit.
Different solutions I can think of:
message containing the context.
could have a special path for exception types deriving from
BaseException: directly call the constructor.
nnorwitz commentedon Apr 5, 2008
Brett, didn't you have a similar problem you fixed a while ago? I
assigned to you for more input, feel free to reset to nobody.
brettcannon commentedon Apr 5, 2008
Neal has the memory of an elephant or something.
Yes, I dealt with a similar issue where the recursion limit was hit, but
then normalizing the exception just caused it to hit it again. I thought
I changed it such that normalizing an exception actually turned off the
depth check or to raise a pre-defined exception for the recursion depth
limit.
I don't have time to look at this right now (still on holiday in
Brussels), but if I remember correctly the last time this came up I
liked the pre-allocated recursion limit exception.
devdanzin commentedon Apr 6, 2008
I've identified rev58032 [1] as the one introducing this issue. It's
Brett's code, fixing a nasty crasher and adding a pre-built exception
(PyExc_RecursionErrorInst).
[1] http://svn.python.org/view?rev=58032&view=rev
P.S.: Thanks Thomas for correctly identifying an issue I mis-reported :)
warsaw commentedon May 8, 2008
I'm not going to hold up the 2.6 alpha release for this, but will bump
it up for the first 2.6 beta.
warsaw commentedon Jun 11, 2008
This is a bug that can be fixed after beta, so I'm knocking it back to
critical for beta 1.
benjaminp commentedon Jul 31, 2008
Ping
devdanzin commentedon Aug 13, 2008
Contrary to (what I understand from) Amaury's analysis, ISTM that the
call to PyErr_SetObject is the problem, as after the recursion limit is
hit PyErr_NormalizeException isn't called again.
Commenting off the PyErr_SetObject line suppresses the "undetected
errors" and passes the unittests (including the infinite recursion
crashers removed in that rev). I have no idea about the problems it may
cause, though.
brettcannon commentedon Aug 14, 2008
On Wed, Aug 13, 2008 at 12:49 PM, Daniel Diniz <[email protected]> wrote:
If I remember correctly, that is on purpose as normalizing the
exception could lead to the stack being blown again. But this totally
off of memory, so I could be wrong.
pitrou commentedon Aug 21, 2008
For what it's worth, py3k has a subtler recursion checking algorithm
which would probably fix this problem if backported properly. See
_Py_CheckRecursiveCall() in ceval.c (lines 462+), and especially the
role played by the tstate->overflowed flag, which allows a moderate
overflow of the recursion count in order for error handling code to
execute properly.
pitrou commentedon Aug 29, 2008
The crashers which were deleted in rev58032 should at least have been
turned into unittests... well.
pitrou commentedon Aug 30, 2008
Ok, here is a patch which seems to cover all bases. It fixes the present
bug, adds tests for the aforementioned ex-crashers, backports the
somewhat smarter recursion checking from py3k, and improves the latter
by fixing a nasty recursion bug when exceptions are ignored by C code.
I have to explain the latter problem a bit. Some functions like
PyDict_GetItem() are designed to silence all exceptions. If a
RuntimeError is raised because of a recursion overflow, however, a flag
(named "overflowed") is additionally set in the thread state structure;
this flag temporarily bumps the recursion_limit by 50, so that the code
which caught the exception can perform some cleanup without itself
getting into the recursion limit. However, if recursion_limit + 50 is in
turned reached, the interpreter aborts with a fatal error.
Now, if the RuntimeError is discarded by PyDict_GetItem(), the
"overflowed" flag must also be reset, because the caller won't know that
there was a recursion overflow and that it must clean up. Instead, it
will simply assume the requested value is not in the dict, and will
continue happily... until it overflows the recursion limit again, which
will trigger the aforementioned fatal error if "overflowed" has not been
reset.
Consequently, PyErr_Clear() now resets the "overflowed" flag if the
current recursion count has gone back below the recursion limit. This
improvement must also be merged back to py3k.
Of course, we may also decide that all those subtle changes are not
worth the risk, just to fix a few obscure glitches.
pitrou commentedon Aug 30, 2008
Ouch, there were a couple of useless lines in ceval.h. New patch.
pitrou commentedon Aug 30, 2008
After thinking about it a bit, I think the whole recursion checking
thing has gone a bit mad. It probably deserves proper discussion on the
mailing-list. In the meantime, I'm downgrading this bug to critical.
jafo commentedon Mar 20, 2010
The final word on this seems to be this:
Wait until when we aren't in a beta release. (DONE)
Quoting:
Well, For Py3K at least we might need to consider going through the C
API and fixing it so that these incorrect assumptions that functions
like PyDict_GetItem() make are no longer made by introducing some new
functions that behave in a "better" way.
And for the recursion issue, I think it stems from corners that are
cut in the C API by us. We inline functions all over the place, assume
that Python's implementation underneath the hood is going to make
calls that stay in C, etc. But as time has gone on and we have added
flexibility to Python, more and more places have a chance to call
Python code and trigger issues.
from Brett's e-mail at:
http://mail.python.org/pipermail/python-dev/2008-August/082107.html
Anyone up to trying to make the C API changes required to get this
resolved?
Sean
BreamoreBoy commentedon Jul 13, 2010
This has been fixed in Python 2.7, I suppose that the code could be backported to 2.6, or is it now too late so this can be closed as "won't fix"?
brettcannon commentedon Jul 13, 2010
It theoretically could be backported so it can stay open for now if someone feels adventurous to backport before 2.6.6. Once that's out the door, though, this can get closed.