[PATCH] Fix refcounting bug in typecaster

Michael Tharp <gxti-1pawZKhx9Om5WRpDikjj11aTQe2KTcn/@public.gmane.org> Wed, 07 Apr 2010 15:19:46 -0400

Newsgroups	gmane.comp.python.db.psycopg.devel
Message-ID	<[email protected]>

Greetings!

I have found the cause of the assertion error from earlier. The key was 
getting it through my insufficiently-caffeinated skull that the crash 
was happening in the middle of a call to the caster function, where a 
new object being instantiated triggered a garbage collection. The GC 
then trips over a too-small refcount on the type object which is 
reachable twice even though the refcnt is only 1.

Here's a test case, I don't know if you want to include it in the test 
suite so I didn't write one.
---SNIP---
import gc
import psycopg2
from psycopg2 import extensions

db = psycopg2.connect('')

def caster(val, cur):
     gc.collect()
     return 42

typ = extensions.new_type((2950,), "UUID", caster)
extensions.register_type(typ, db)
cu = db.cursor()
cu.execute("select 'b5219e01-19ab-4994-b71e-149225dc51e4'::uuid")
cu.fetchone()
---SNIP---

Moral of the story: Don't Py_VISIT objects via a stolen reference.

Patch is attached. Keep up the great work!

-- m. tharp

_______________________________________________
Psycopg mailing list
Psycopg-IAPFreCvJWPBWskQ1e/[email protected]
http://lists.initd.org/mailman/listinfo/psycopg

0001-Fix-refcounting-bug-in-typecaster.patch (text/plain, 1.7 KB)

From 5e5d63544675df91a53cfbee7d9d5c287b10d23b Mon Sep 17 00:00:00 2001
From: Michael Tharp <gxti-1pawZKhx9Om5WRpDikjj11aTQe2KTcn/@public.gmane.org>
Date: Wed, 7 Apr 2010 15:01:57 -0400
Subject: [PATCH] Fix refcounting bug in typecaster

This could cause an assertion if a garbage collection happened in the
middle of a caster function, which is quite probable since caster
functions are usually allocating new instances.
---
 psycopg/typecast.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/psycopg/typecast.c b/psycopg/typecast.c
index 363196c..326d3ea 100644
--- a/psycopg/typecast.c
+++ b/psycopg/typecast.c
@@ -381,9 +381,12 @@ typecast_dealloc(PyObject *obj)
 {
     typecastObject *self = (typecastObject*)obj;
 
+    PyObject_GC_UnTrack(self);
+
     Py_CLEAR(self->values);
     Py_CLEAR(self->name);
     Py_CLEAR(self->pcast);
+    Py_CLEAR(self->bcast);
 
     obj->ob_type->tp_free(obj);
 }
@@ -396,6 +399,7 @@ typecast_traverse(PyObject *obj, visitproc visit, void *arg)
     Py_VISIT(self->values);
     Py_VISIT(self->name);
     Py_VISIT(self->pcast);
+    Py_VISIT(self->bcast);
     return 0;
 }
 
@@ -587,7 +591,7 @@ typecast_cast(PyObject *obj, const char *str, Py_ssize_t len, PyObject *curs)
     PyObject *old, *res = NULL;
     typecastObject *self = (typecastObject *)obj;
 
-    /* we don't incref, the caster *can't* die at this point */
+    Py_INCREF(obj);
     old = ((cursorObject*)curs)->caster;
     ((cursorObject*)curs)->caster = obj;
 
@@ -602,6 +606,7 @@ typecast_cast(PyObject *obj, const char *str, Py_ssize_t len, PyObject *curs)
     }
 
     ((cursorObject*)curs)->caster = old;
+    Py_DECREF(obj);
 
     return res;
 }
-- 
1.6.6.1