classification
Title: Try to print repr() when an C-level assert fails (in the garbage collector, beyond?)
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.3
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: bkabrda, dmalcolm, loewis, ncoghlan, pitrou
Priority: normal Keywords: patch

Created on 2010-07-14 23:50 by dmalcolm, last changed 2013-06-10 19:53 by bkabrda.

Files
File name Uploaded Description Edit
py3k-repr-on-gcmodule-assertions.patch dmalcolm, 2010-07-14 23:50 review
py3k-objdump-on-gcmodule-assertions-2010-11-22-001.patch dmalcolm, 2010-11-22 22:13 review
py3k-objdump-on-gcmodule-assertions-2011-01-10-001.patch dmalcolm, 2011-01-10 23:57 review
py3k-objdump-on-gcmodule-assertions-2011-01-10-002.patch dmalcolm, 2011-01-10 23:59 review
00170-gc-assertions.patch bkabrda, 2013-06-10 19:53
Messages (7)
msg110341 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2010-07-14 23:50
Modules/gcmodule.c contains various assertions which can fail due to reference counting errors elsewhere in either python, or an extension module.  These can be difficult to track down.

In the hope of maximizing the information from crash reports, the attached patch (against py3k) introduces a new assertion macro to Objects/object.h and Objects.c, which provides a richer debug message.  In particular, it identifies which object has the issue, and can more clearly spell out the problem.

The patch replaces all uses of assert() in Modules/gcmodule.c for which a specific object has an issue (e.g. bogus reference count).

The implementation may play somewhat fast-and-loose with rules about object invariants: you might have an only partially valid object, but the process is about to abort, so it seems acceptable to try to glean extra information on stderr.  (This may turn an abort into a segfault, of course)

Caveats:
- exact name of the API probably could be better
- I don't yet have a specific use for the "callback" idea; I was thinking of trying to display all objects that reference that object.  Might need a void* closure to be useful.  Might be a useless complication.
- Only tested on gcc-4.4.3 so far; the __STRING(expr) and __PRETTY_FUNCTION__ look non-portable.
- no test case; I thought about using ctypes to extract PyObject_IncRef from the implementation, but this is likely to lead to brittle test cases.  Alternatively, is xxmodule to be used for this kind of thing?

Thoughts?
msg122170 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2010-11-22 22:13
Attaching a simplified version of the patch; I got rid of the callbacks.

Still doesn't have test cases.

I suspect that the use of __STRING and __PRETTY_FUNCTION__ may be compatibility issues.  I believe that __FILE__ and __LINE__ and standard C though.
msg125959 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2011-01-10 23:57
Attaching updated version of the patch.

I've added a selftest which (in a sacrificial subprocess) abuses ctypes to break an ob_refcnt, and then triggers a garbage collection.

I also changed the printing to stderr to directly use fprintf and fflush to ensure that data leaves the process before abort kills it (not sure if this is a cross-platform or unicode no-no, though).
msg125960 - (view) Author: Dave Malcolm (dmalcolm) (Python committer) Date: 2011-01-10 23:59
As above, but I added an extra call to fflush in case the call to _PyObject_Dump leads to a segfault.
msg131016 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-15 18:39
How about turning these asserts into Py_FatalError()s and then enabling Victor's faulthandler extension?
msg131036 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-03-15 20:36
I'd suggest calling Py_FatalError rather than calling abort() directly in _PyObject_AssertFailed, but otherwise this looks like a nice improvement over standard C asserts for state invariants that may be broken by buggy C extensions.

For the tests, take a look at test.script_helper - it provides some convenience wrappers for spawning subprocesses for tests that would cause problems if run in the current process.
msg190929 - (view) Author: Bohuslav "Slavek" Kabrda (bkabrda) * Date: 2013-06-10 19:53
I'm currently patching Python 3.3.2 with this, so I thought it might be nice to attach an up-to-date patch. The only notable difference is that I added self.preclean() at the beginning of test_refcount_errors - without it, running test suite produced a huge number of these lines:

Exception AttributeError: "'GCCallbackTests' object has no attribute 'visit'" in <bound method GCCallbackTests.cb1 of <__main__.GCCallbackTests testMethod=test_refcount_errors>> ignored
History
Date User Action Args
2013-06-10 19:53:31bkabrdasetfiles: + 00170-gc-assertions.patch
nosy: + bkabrda
messages: + msg190929

2011-03-15 20:36:39ncoghlansetnosy: + ncoghlan
messages: + msg131036
2011-03-15 18:39:23pitrousetnosy: loewis, pitrou, dmalcolm
messages: + msg131016
2011-03-15 18:28:49dmalcolmsetnosy: + loewis
2011-01-10 23:59:57dmalcolmsetfiles: + py3k-objdump-on-gcmodule-assertions-2011-01-10-002.patch
nosy: pitrou, dmalcolm
messages: + msg125960
2011-01-10 23:57:33dmalcolmsetfiles: + py3k-objdump-on-gcmodule-assertions-2011-01-10-001.patch
nosy: pitrou, dmalcolm
messages: + msg125959
2011-01-03 20:07:22pitrousetnosy: pitrou, dmalcolm
versions: + Python 3.3, - Python 3.2
2010-11-22 22:13:23dmalcolmsetfiles: + py3k-objdump-on-gcmodule-assertions-2010-11-22-001.patch

messages: + msg122170
2010-11-22 21:59:37dmalcolmsetnosy: + pitrou
2010-07-14 23:50:49dmalcolmcreate