[egenix-users] mx base 2.1.0b5 ref count bug?

[David Rushby]

--- "M.-A. Lemburg" <mal at egenix.com> wrote:
> I'm not sure whether your analysis of the problem is correct...

Indeed, my original analysis wasn't correct; I think this one is.

The fatal error I'm complaining about only arises if the arguments to
DateTime.DateTime are invalid, so that the object is deallocated in the
onError block of mxDateTime_FromDateAndTime.  If the object is created
successfully, and later deallocated by Python's garbage collector, the
fatal error doesn't arise.

Here's my analysis of the problem:

---

If the debugging directive Py_TRACE_REFS is enabled, as it is in my
case, the Python interpreter maintains a doubly linked list of all
Python objects (named refchain) via the _ob_next and _ob_prev pointers
added to PyObject via _PyObject_HEAD_EXTRA.

Each object is inserted into refchain by the _Py_AddToAllObjects call
in _Py_NewReference, and removed from refchain by the
_Py_ForgetReference call in _Py_Dealloc.

_Py_ForgetReference performs an integrity check on the object being
deallocated (op) to make sure that its neighbors in refchain recognize
it.  Specifically, _Py_ForgetReference verifies that the _ob_next
pointer of the previous object in refchain (op->_ob_prev->_ob_next)
points to op, and that the _ob_prev pointer of the next object in
refchain (op->_ob_next->_ob_prev) points to op.  If either of those
conditions is false, the interpreter raises the fatal error I'm seeing.

After mxDateTime_New pulls an mxDateTimeObject out of the free list, it
calls _Py_NewReference on that object to transform it from a
"conceptually deallocated" object to a "conceptually allocated" one. 
_Py_NewReference inserts the object into refchain.

Later, when the object is being "conceptually deallocated", it should
be removed from refchain with _Py_ForgetReference.  The version of
_Py_Dealloc that's active when Py_TRACE_REFS is enabled does this
(_Py_Dealloc is the element of Py_DECREF that performs the actual
destruction).  _Py_Dealloc first calls _Py_ForgetReference to remove
the object from refchain, then calls the object's destructor
(op->ob_type->tp_dealloc).  If WANT_SUBCLASSABLE_TYPES is not defined
(it is not in my case), the destructor for mxDateTime_Type is
mxDateTime_Free.

So, if a DateTime object is deallocated normally, as a result of its
reference count falling to zero, _Py_Dealloc is called on the object,
which first removes it from refchain, then passes it to
mxDateTime_Free.

If a DateTime object is destroyed prematurely, as a result of invalid
arguments to its destructor, the onError handler calls mxDateTime_Free,
so the object is "conceptually deallocated" (placed in the free list),
*but* the object is not removed from refchain.  So if a
refchain-membership-integrity-check is performed on the object in
question or one of its immediate neighbors in refchain, that integrity
check fails, leading to the fatal error.

---

The solution is to 'Py_DECREF(...)' the object in the onError handler
of its constructor, instead of passing the object directly to
mxDateTime_Free.  That change solves two problems:

  1) It causes _Py_Dealloc to execute, which in turn executes
_Py_ForgetReference, so the object is removed from refchain, as it
should be.

  2) At present, objects that are created successfully and later
deallocated by normal reference count mechanics have a reference count
of 0 when they enter the free list, whereas objects whose constructors
abort their creation due to invalid arguments have a reference count of
1 when they enter the free list.
     Although AFAIK this doesn't cause an actual execution problem
(because the object's reference count is *explicitly set to 1* rather
than *incremented by 1* when the object is later "conceptually
allocated" by _Py_NewReference), it is likely to confuse future
maintainers and thereby lead to the introduction of an actual execution
problem.

I assume that if you agree that this is a reasonable solution, you'll
want to apply the change not only to mxDateTime_FromDateAndTime, but to
all similar constructors in mxDateTime.c.

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com