There were no comments explaining what Py_CLEAR() did or

why it's important.  Now there are ;-)

If someone else hasn't already, I'll add a Py_CLEAR cleanup
task to the TODO Wiki next.

diff --git a/Include/object.h b/Include/object.h
index 924480f888640e9fdd3b453df7c1074b61430bf4..c6d0fc39516f0e386cb08a323cb2c629d023201b 100644 (file)
--- a/Include/object.h
+++ b/Include/object.h
@@ -645,6 +645,40 @@ PyAPI_FUNC(void) _Py_AddToAllObjects(PyObject *, int force);
        else                                            \
                _Py_Dealloc((PyObject *)(op))
 
+/* Safely decref `op` and set `op` to NULL, especially useful in tp_clear
+ * and tp_dealloc implementatons.
+ *
+ * Note that "the obvious" code can be deadly:
+ *
+ *     Py_XDECREF(op);
+ *     op = NULL;
+ *
+ * Typically, `op` is something like self->containee, and `self` is done
+ * using its `containee` member.  In the code sequence above, suppose
+ * `containee` is non-NULL with a refcount of 1.  Its refcount falls to
+ * 0 on the first line, which can trigger an arbitrary amount of code,
+ * possibly including finalizers (like __del__ methods or weakref callbacks)
+ * coded in Python, which in turn can release the GIL and allow other threads
+ * to run, etc.  Such code may even invoke methods of `self` again, or cause
+ * cyclic gc to trigger, but-- oops! --self->containee still points to the
+ * object being torn down, and it may be in an insane state while being torn
+ * down.  This has in fact been a rich historic source of miserable (rare &
+ * hard-to-diagnose) segfaulting (and other) bugs.
+ *
+ * The safe way is:
+ *
+ *      Py_CLEAR(op);
+ *
+ * That arranges to set `op` to NULL _before_ decref'ing, so that any code
+ * triggered as a side-effect of `op` getting torn down no longer believes
+ * `op` points to a valid object.
+ *
+ * There are cases where it's safe to use the naive code, but they're brittle.
+ * For example, if `op` points to a Python integer, you know that destroying
+ * one of those can't cause problems -- but in part that relies on that
+ * Python integers aren't currently weakly referencable.  Best practice is
+ * to use Py_CLEAR() even if you can't think of a reason for why you need to.
+ */
 #define Py_CLEAR(op)                           \
         do {                                   \
                 if (op) {                      \