traceback.clear_frames(): Objects/typeobject.c:3086: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed. #78249
Comments
|
I got the following failure when running tests with coverage on VSTS. https://python.visualstudio.com/cpython/cpython%20Team/_build/results?buildId=14680&view=logs 2018-07-05T12:11:22.0719913Z 0:17:49 load avg: 0.82 [373/413] test_urllib It is not related to changes made in PR 8071. Is Coverage.py doing something wrong or just exposes a flaw in the interpreter? |
serhiy-storchaka
added
3.8
|
I added this assertion exactly for the purpose of finding this kind of bug. It means that some code tried to look up an attribute with a live exception set, which previously could swallow the exception in certain situations, and even if not, it is always the wrong thing to do. Looking at the stack trace, my guess is that the problem is closer to the crash than the coverage calls. I'd suggest taking a look at the frame cleanup in the traceback module. |
vstinner
changed the title
|
I'm able to reproduce the bug on my Fedora using: vstinner@apu$ gdb -args ENV/bin/python -m coverage run --pylib -m test --fail-env-changed -uall,-cpu test_urllib -F python: Objects/typeobject.c:3086: _PyType_Lookup: Assertion `!PyErr_Occurred()' failed. Program received signal SIGABRT, Aborted. (gdb) py-bt
Traceback (most recent call first):
<built-in method close of HTTPResponse object at remote 0x7fffe581ff50>
File "/home/vstinner/prog/python/master/Lib/http/client.py", line 405, in close
super().close() # set "closed" flag
File "/home/vstinner/prog/python/master/Lib/traceback.py", line 216, in clear_frames
tb.tb_frame.clear()
File "/home/vstinner/prog/python/master/Lib/unittest/case.py", line 205, in __exit__
traceback.clear_frames(tb)
File "/home/vstinner/prog/python/master/Lib/unittest/case.py", line 178, in handle
callable_obj(*args, **kwargs)
File "/home/vstinner/prog/python/master/Lib/unittest/case.py", line 743, in assertRaises
return context.handle('assertRaises', args, kwargs)
File "/home/vstinner/prog/python/master/Lib/test/test_urllib.py", line 382, in test_redirect_limit_independent
"http://something")
(...)(gdb) where The bug is in _io__IOBase_close_impl(). I modified the code: res = PyObject_CallMethodObjArgs(self, _PyIO_str_flush, NULL); if (_PyObject_SetAttrId(self, &PyId___IOBase_closed, Py_True) < 0) {
Py_XDECREF(res);
return NULL;
}The problem is that _PyObject_SetAttrId() is called while the flush() method raised an exception. We need something like PyErr_Fetch/PyErr_Store to set the attribute. |
|
Stefan Behnel: "I added this assertion exactly for the purpose of finding this kind of bug. It means that some code tried to look up an attribute with a live exception set, which previously could swallow the exception in certain situations, and even if not, it is always the wrong thing to do." The assertion failure is a little bit "far" from the bug: would it make sense to add "assert(!PyErr_Occurred());" to the entry point of:
A few years ago, I added "assert(!PyErr_Occurred());" to the entry point of C functions which can call arbitrary code like _PyEval_EvalFrameDefault(), PyObject_Call(), etc. It helped to find many bugs. |
Using this additional assertion, test_io also fails on test_flush_error_on_close(). |
|
_PyType_Lookup doesn't set an exception. But if an exception was raised inside, it will be cleared, and this will clear an exception if it was raised before _PyType_Lookup. assert() is needed for guaranteeing that no exception will be lost. I'm +1 for saving/restoring (or rather chaining) the exception. |
Stefan, Serhiy: any opinion on my idea? For example, PyObject_HasAttr() can replace and clear the current exception *by design*. This function does more or less: try: obj.attr; return True int
PyObject_HasAttr(PyObject *v, PyObject *name)
{
PyObject *res;
if (_PyObject_LookupAttr(v, name, &res) < 0) {
PyErr_Clear();
return 0;
}
if (res == NULL) {
return 0;
}
Py_DECREF(res);
return 1;
} |
|
Python 2.7 also has the io module: it may be good to backport the change to Python 2.7 as well, no? I see the same bug in 2.7: res = PyObject_CallMethodObjArgs(self, _PyIO_str_flush, NULL);
PyObject_SetAttrString(self, "__IOBase_closed", Py_True);
if (res == NULL) {
return NULL;
} |
This looks like a part of larger issue, and I think it deserves a separate issue on the tracker. See also bpo-26776. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: