From 9fec8f59f64599cd5c65363cfa752d4603f480f8 Mon Sep 17 00:00:00 2001 From: lericson Date: Thu, 26 Nov 2009 13:19:50 +0100 Subject: [PATCH] Fix crashes in client allocation and deallocation routines. --- _pylibmcmodule.c | 30 +++++++++++++++++------------- _pylibmcmodule.h | 32 ++++++++++++++++---------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/_pylibmcmodule.c b/_pylibmcmodule.c index 3910f18..85d1571 100644 --- a/_pylibmcmodule.c +++ b/_pylibmcmodule.c @@ -40,18 +40,23 @@ static PylibMC_Client *PylibMC_ClientType_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { PylibMC_Client *self; - self = (PylibMC_Client *)type->tp_alloc(type, 0); + /* GenericNew calls GenericAlloc (via the indirection type->tp_alloc) which + * adds GC tracking if flagged for, and also calls PyObject_Init. */ + self = (PylibMC_Client *)PyType_GenericNew(type, args, kwds); + if (self != NULL) { self->mc = memcached_create(NULL); - } else { - args = kwds = NULL; } return self; } static void PylibMC_ClientType_dealloc(PylibMC_Client *self) { - memcached_free(self->mc); + if (self->mc != NULL) { + memcached_free(self->mc); + } + + self->ob_type->tp_free(self); } /* }}} */ @@ -860,7 +865,7 @@ static PyObject *PylibMC_Client_flush_all(PylibMC_Client *self, rc = memcached_flush(self->mc, expire); if (rc != MEMCACHED_SUCCESS) - return PylibMC_ErrFromMemcached(self, "delete_multi", rc); + return PylibMC_ErrFromMemcached(self, "flush_all", rc); Py_RETURN_TRUE; } @@ -871,18 +876,17 @@ static PyObject *PylibMC_Client_disconnect_all(PylibMC_Client *self) { } static PyObject *PylibMC_Client_clone(PylibMC_Client *self) { - PylibMC_Client *new_self; + /* Essentially this is a reimplementation of the allocator, only it uses a + * cloned memcached_st for mc. */ + PylibMC_Client *clone; - /* XXX Is it really wise to short-circuit like this? I don't see a problem - * with it. */ - new_self = (PylibMC_Client *)self->ob_type->tp_new(self->ob_type, NULL, NULL); - if (new_self == NULL) { + clone = (PylibMC_Client *)PyType_GenericNew(self->ob_type, NULL, NULL); + if (clone == NULL) { return NULL; } - memset(new_self->mc, 0, sizeof(memcached_st)); - new_self->mc = memcached_clone(new_self->mc, self->mc); - return (PyObject *)new_self; + clone->mc = memcached_clone(NULL, self->mc); + return (PyObject *)clone; } /* }}} */ diff --git a/_pylibmcmodule.h b/_pylibmcmodule.h index d2143f4..252700c 100644 --- a/_pylibmcmodule.h +++ b/_pylibmcmodule.h @@ -289,23 +289,23 @@ static PyTypeObject PylibMC_ClientType = { 0, 0, PylibMC_ClientType_methods, - 0, - 0, - 0, - 0, - 0, - 0, - 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, (initproc)PylibMC_Client_init, - 0, - (newfunc)PylibMC_ClientType_new, - 0, - 0, - 0, - 0, - 0, - 0, - 0, + 0, + (newfunc)PylibMC_ClientType_new, //PyType_GenericNew, + 0, + 0, + 0, + 0, + 0, + 0, + 0, 0 };