python / cpython

The Python programming language
https://www.python.org
Other
63.49k stars 30.41k forks source link

Document that tp_dealloc handler must call PyObject_GC_UnTrack if Py_TPFLAGS_HAVE_GC is set #72923

Closed colesbury closed 3 years ago

colesbury commented 7 years ago
BPO 28737
Nosy @ambv, @colesbury, @miss-islington
PRs
  • python/cpython#29246
  • python/cpython#29248
  • python/cpython#29249
  • 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: ```python assignee = 'https://github.com/colesbury' closed_at = created_at = labels = ['extension-modules', '3.11', '3.9', '3.10', 'docs'] title = 'Document that tp_dealloc handler must call PyObject_GC_UnTrack if Py_TPFLAGS_HAVE_GC is set' updated_at = user = 'https://github.com/colesbury' ``` bugs.python.org fields: ```python activity = actor = 'lukasz.langa' assignee = 'colesbury' closed = True closed_date = closer = 'lukasz.langa' components = ['Documentation', 'Extension Modules'] creation = creator = 'colesbury' dependencies = [] files = [] hgrepos = [] issue_num = 28737 keywords = ['patch'] message_count = 6.0 messages = ['281146', '405112', '405120', '405232', '405233', '405234'] nosy_count = 4.0 nosy_names = ['docs@python', 'lukasz.langa', 'colesbury', 'miss-islington'] pr_nums = ['29246', '29248', '29249'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue28737' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    colesbury commented 7 years ago

    In general, an a PyTypeObject that has Py_TPFLAGS_HAVE_GC set must call PyObject_GC_UnTrack() before it frees any PyObject* references it owns. The only reference to this requirement I found is in https://docs.python.org/3/c-api/gcsupport.html#c.\_PyObject_GC_TRACK.

    This requirement should be documented in:

    1. https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dealloc
    2. https://docs.python.org/3/extending/newtypes.html

    A call to PyObject_GC_UnTrack() should also be added to he official "noddy4" example. Currently, the example is incorrect and can crash if a referred-to object triggers a GC from it's destructor. See the following example which segfaults:

    https://github.com/colesbury/noddy

    It may be worthwhile to have _Py_Dealloc call PyObject_GC_UnTrack() if the PyTypeObject has Py_TPFLAGS_HAVE_GC set. Considering that the official Python extension example is missing the call, it seems likely that extension writers often forget to include it.

    colesbury commented 3 years ago

    Antoine Pitrou already fixed the "noddy4" example (now renamed to "custom4") and updated the newtypes_tutorial, but I think it's still worth mentioning PyObject_GC_Untrack in a few additional places.

    ambv commented 3 years ago

    New changeset 35e1ff38ee67ee543d9fcb268c3552c5397f9b3f by Sam Gross in branch 'main': bpo-28737: Document when tp_dealloc should call PyObject_GC_UnTrack() (GH-29246) https://github.com/python/cpython/commit/35e1ff38ee67ee543d9fcb268c3552c5397f9b3f

    ambv commented 3 years ago

    New changeset 9e0012116ac9e8d26bf19ef8741deeecf2b6f72b by Sam Gross in branch '3.10': [3.10] bpo-28737: Document when tp_dealloc should call PyObject_GC_UnTrack() (GH-29246) (GH-29249) https://github.com/python/cpython/commit/9e0012116ac9e8d26bf19ef8741deeecf2b6f72b

    ambv commented 3 years ago

    New changeset 193504acf3bfb7cff1edf7f568c2405b857fa1f7 by Miss Islington (bot) in branch '3.9': bpo-28737: Document when tp_dealloc should call PyObject_GC_UnTrack() (GH-29246) (GH-29248) https://github.com/python/cpython/commit/193504acf3bfb7cff1edf7f568c2405b857fa1f7

    ambv commented 3 years ago

    Thanks, Sam! ✨ 🍰 ✨