classification
Title: Debug mode: check if objects are valid
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: methane, pablogsal, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2020-03-06 09:24 by vstinner, last changed 2020-03-25 16:28 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 18803 closed vstinner, 2020-03-06 09:36
PR 18804 merged vstinner, 2020-03-06 09:39
PR 18807 merged vstinner, 2020-03-06 15:13
Messages (5)
msg363499 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-06 09:24
One of the worst issue that I had to debug is a crash in the Python garbage collector. It is usually a crash in visit_decref(). See my notes:
https://pythondev.readthedocs.io/debug_tools.html#debug-crash-in-garbage-collection-visit-decref

My previous attempt to help debugging such issue failed: bpo-36389 "Add gc.enable_object_debugger(): detect corrupted Python objects in the GC". The idea was to check frequently if all objects tracked by the GC are valid. The problem is that even if the check looked trivial, checking all objects made Python way slower. Even when I tried to only check objects of the "young" GC generation (generation 0), it was still too slow.

Here I propose a different approach: attempt to only check objects when they are accessed. Recently, I started to replace explicit cast to (PyObject *) type with an indirection: a new _PyObject_CAST() macro which should be the only way to cast any object pointer to (PyObject *).

/* Cast argument to PyObject* type. */
#define _PyObject_CAST(op) ((PyObject*)(op))

This macro is used in many "core" macros like Py_TYPE(op), Py_REFCNT(op), Py_SIZE(op), Py_SETREF(op, op2), Py_VISIT(op), etc.

The idea here is to inject code in _PyObject_CAST(op) when Python is built in debug mode to ensure that the object is valid. The intent is to detect corrupted objects earlier than a garbage collection, to ease debugging C extensions.

The checks should be limited to reduce the performance overhead.

Attached PR implemnts this idea.
msg363500 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-06 09:40
See also bpo-38392: "Ensure that objects entering the GC are valid". I fixed this one with commit 1b1845569539db5c1a6948a5d32daea381f1e35f.
msg363521 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-06 14:55
New changeset 1fb5a9f394a6fdf62e21b96080c3257c959cf8c9 by Victor Stinner in branch 'master':
bpo-39873: PyObject_Init() uses PyObject_INIT() (GH-18804)
https://github.com/python/cpython/commit/1fb5a9f394a6fdf62e21b96080c3257c959cf8c9
msg363529 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-06 17:57
New changeset 9a73705a1d0cb8b89d0a20add2ffa2c4d32950ed by Victor Stinner in branch 'master':
bpo-39873: Cleanup _PyObject_CheckConsistency() (GH-18807)
https://github.com/python/cpython/commit/9a73705a1d0cb8b89d0a20add2ffa2c4d32950ed
msg364997 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-25 16:28
This change was mostly an experimentation. But I'm not sure that it does solve any issue. The change is more intrusive than what I expected. I prefer to abandon PR 18803 and this issue.
History
Date User Action Args
2020-03-25 16:28:32vstinnersetstatus: open -> closed
resolution: rejected
messages: + msg364997

stage: patch review -> resolved
2020-03-06 17:57:51vstinnersetmessages: + msg363529
2020-03-06 15:13:13vstinnersetpull_requests: + pull_request18165
2020-03-06 14:55:35vstinnersetmessages: + msg363521
2020-03-06 09:41:09vstinnersetnosy: + methane, serhiy.storchaka, pablogsal
2020-03-06 09:40:54vstinnersetmessages: + msg363500
2020-03-06 09:39:24vstinnersetpull_requests: + pull_request18160
2020-03-06 09:36:08vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request18159
2020-03-06 09:24:50vstinnercreate