[Python-checkins] r45411 - python/trunk/Include/object.h

[tim.peters]

Author: tim.peters
Date: Sat Apr 15 04:14:03 2006
New Revision: 45411

Modified:
   python/trunk/Include/object.h
Log:
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.


Modified: python/trunk/Include/object.h
==============================================================================
--- python/trunk/Include/object.h	(original)
+++ python/trunk/Include/object.h	Sat Apr 15 04:14:03 2006
@@ -645,6 +645,40 @@
 	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) {			\