From 9d2ced8fa040bb16667b65b3aa0e9385612f4a0e Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Sat, 15 Apr 2006 02:14:03 +0000 Subject: 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. --- Include/object.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Include/object.h b/Include/object.h index 924480f..c6d0fc3 100644 --- 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) { \ -- cgit v0.12