From d4ae97bc38780aab5f348b73fee67eaab7546441 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 29 Aug 2008 18:39:48 +0000 Subject: [PATCH] #3668: When PyArg_ParseTuple correctly parses a s* format, but raises an exception afterwards (for a subsequent parameter), the user code will not call PyBuffer_Release() and memory will leak. Reviewed by Amaury Forgeot d'Arc. --- Include/cobject.h | 9 ++++++++ Misc/NEWS | 4 ++++ Objects/cobject.c | 7 ------ Python/getargs.c | 56 ++++++++++++++++++++++++++++++++++------------- 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/Include/cobject.h b/Include/cobject.h index 499dfadddfe..4e24d7dcd62 100644 --- a/Include/cobject.h +++ b/Include/cobject.h @@ -48,6 +48,15 @@ PyAPI_FUNC(void *) PyCObject_Import(char *module_name, char *cobject_name); /* Modify a C object. Fails (==0) if object has a destructor. */ PyAPI_FUNC(int) PyCObject_SetVoidPtr(PyObject *self, void *cobj); + +typedef struct { + PyObject_HEAD + void *cobject; + void *desc; + void (*destructor)(void *); +} PyCObject; + + #ifdef __cplusplus } #endif diff --git a/Misc/NEWS b/Misc/NEWS index 822d1c956a4..34fbd69a9e9 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,10 @@ What's New in Python 2.6 release candidate 1? Core and Builtins ----------------- +- Issue #3668: Fix a memory leak with the "s*" argument parser in + PyArg_ParseTuple and friends, which occurred when the argument for "s*" + was correctly parsed but parsing of subsequent arguments failed. + - Issue #2534: speed up isinstance() and issubclass() by 50-70%, so as to match Python 2.5 speed despite the __instancecheck__ / __subclasscheck__ mechanism. In the process, fix a bug where isinstance() and issubclass(), diff --git a/Objects/cobject.c b/Objects/cobject.c index a1ee68622ad..c437491c27c 100644 --- a/Objects/cobject.c +++ b/Objects/cobject.c @@ -9,13 +9,6 @@ typedef void (*destructor1)(void *); typedef void (*destructor2)(void *, void*); -typedef struct { - PyObject_HEAD - void *cobject; - void *desc; - void (*destructor)(void *); -} PyCObject; - PyObject * PyCObject_FromVoidPtr(void *cobj, void (*destr)(void *)) { diff --git a/Python/getargs.c b/Python/getargs.c index 766a2d70070..9d1f0b218ba 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -139,24 +139,35 @@ _PyArg_VaParse_SizeT(PyObject *args, char *format, va_list va) /* Handle cleanup of allocated memory in case of exception */ +static void +cleanup_ptr(void *ptr) +{ + PyMem_FREE(ptr); +} + +static void +cleanup_buffer(void *ptr) +{ + PyBuffer_Release((Py_buffer *) ptr); +} + static int -addcleanup(void *ptr, PyObject **freelist) +addcleanup(void *ptr, PyObject **freelist, void (*destr)(void *)) { PyObject *cobj; if (!*freelist) { *freelist = PyList_New(0); if (!*freelist) { - PyMem_FREE(ptr); + destr(ptr); return -1; } } - cobj = PyCObject_FromVoidPtr(ptr, NULL); + cobj = PyCObject_FromVoidPtr(ptr, destr); if (!cobj) { - PyMem_FREE(ptr); + destr(ptr); return -1; } if (PyList_Append(*freelist, cobj)) { - PyMem_FREE(ptr); Py_DECREF(cobj); return -1; } @@ -167,15 +178,15 @@ addcleanup(void *ptr, PyObject **freelist) static int cleanreturn(int retval, PyObject *freelist) { - if (freelist) { - if (retval == 0) { - Py_ssize_t len = PyList_GET_SIZE(freelist), i; - for (i = 0; i < len; i++) - PyMem_FREE(PyCObject_AsVoidPtr( - PyList_GET_ITEM(freelist, i))); - } - Py_DECREF(freelist); + if (freelist && retval != 0) { + /* We were successful, reset the destructors so that they + don't get called. */ + Py_ssize_t len = PyList_GET_SIZE(freelist), i; + for (i = 0; i < len; i++) + ((PyCObject *) PyList_GET_ITEM(freelist, i)) + ->destructor = NULL; } + Py_XDECREF(freelist); return retval; } @@ -798,6 +809,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, if (getbuffer(arg, p, &buf) < 0) return converterr(buf, arg, msgbuf, bufsize); } + if (addcleanup(p, freelist, cleanup_buffer)) { + return converterr( + "(cleanup problem)", + arg, msgbuf, bufsize); + } format++; } else if (*format == '#') { void **p = (void **)va_arg(*p_va, char **); @@ -875,6 +891,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, if (getbuffer(arg, p, &buf) < 0) return converterr(buf, arg, msgbuf, bufsize); } + if (addcleanup(p, freelist, cleanup_buffer)) { + return converterr( + "(cleanup problem)", + arg, msgbuf, bufsize); + } format++; } else if (*format == '#') { /* any buffer-like object */ void **p = (void **)va_arg(*p_va, char **); @@ -1051,7 +1072,7 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, "(memory error)", arg, msgbuf, bufsize); } - if (addcleanup(*buffer, freelist)) { + if (addcleanup(*buffer, freelist, cleanup_ptr)) { Py_DECREF(s); return converterr( "(cleanup problem)", @@ -1096,7 +1117,7 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, return converterr("(memory error)", arg, msgbuf, bufsize); } - if (addcleanup(*buffer, freelist)) { + if (addcleanup(*buffer, freelist, cleanup_ptr)) { Py_DECREF(s); return converterr("(cleanup problem)", arg, msgbuf, bufsize); @@ -1214,6 +1235,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, PyErr_Clear(); return converterr("read-write buffer", arg, msgbuf, bufsize); } + if (addcleanup(p, freelist, cleanup_buffer)) { + return converterr( + "(cleanup problem)", + arg, msgbuf, bufsize); + } if (!PyBuffer_IsContiguous((Py_buffer*)p, 'C')) return converterr("contiguous buffer", arg, msgbuf, bufsize); break;