[Zope-dev] Zope (and Python) crashing bug found

Andy Dustman andy@dustman.net
06 Sep 2001 02:37:05 -0400


There have been a couple of reports from various people (myself
included) about a bug that will cause Zope to mysteriously die and dump
core. I myself had the same problem, particularly with MySQLdb on recent
versions of Python. I decided I better hunt the bastard down sooner
rather than later...

Symptoms: Mysterious core dumps fairly early on after starting up
MySQLdb, isolated to the first query. After some investigation, I
determined that this did not occur with Python 1.5.2 or 2.0, but does
occur with 2.0.1, 2.1, and 2.1.1.

Now, when calling _mysql.connect(), one of the items that you MySQLdb
will pass in is a type conversion dictionary as a keyword argument, i.e.
apply(connect, args, kwargs). After injecting some print statements here
and there, and some getrefcount(), I found that the refcount on the
converter dictionary was going to zero, i.e. getrefcount() returns 1.
However, this only happened with Python > 2.0.

So I got out diff and found the critical difference between 2.0 (MySQLdb
not broken) and 2.0.1 (MySQLdb broken):

diff -u -r Python-2.0/Python/getargs.c Python-2.0.1/Python/getargs.c
--- Python-2.0/Python/getargs.c Mon Oct 16 17:49:29 2000
+++ Python-2.0.1/Python/getargs.c       Sat Mar 31 08:18:35 2001
@@ -1124,6 +1124,7 @@
                                return 0;
                        }
                        converted++;
+                       Py_DECREF(item);
                }
                else {
                        PyErr_Clear();

The general upshot of this is: In versions of Python (numerically) prior
to 2.0.1, if you use the O format with PyArgs_ParseTupleAndKeywords()
(and possibly also PyArgs_ParseTuple()) to return a PyObject *, you get
a new reference. In 2.0.1 and later, you get a borrowed reference and
must Py_INCREF() if you want to keep it. In Python 1.5.2, however, doing
this causes a memory leak! Since I want to keep compatibility with
Python 1.5.2 for now, my solution is this:

	if (!conv) 
		conv = PyDict_New();
#if PY_VERSION_HEX > 0x02000100
	else
		Py_INCREF(conv);
#endif

In this example, conv is initialized to NULL, and
PyArgs_ParseTupleAndKeywords() may or may not set it to some object
pointer if it was passed in.

Since Zope is only going to be supporting 2.1 and newer releases, this
is not necessary. However, it may be worth auditing the code to see if
the O format is being used, and if so, do Py_INCREF()s need to be added.
I suspect some might, since with Python 1.5.2, having them there is a
memory leak.

I'm going to bed now...

-- 
Andy Dustman         PGP: 0x930B8AB6
    @       .net     http://dustman.net/andy
I'll give spammers one bite of the apple, but they'll
have to guess which bite has the razor blade in it.