From f7199578be41182fcbcd0350521f9545210e0ddd Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 29 Aug 2008 18:37:05 +0000 Subject: #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 | 63 +++++++++++++++++++++++++++++++++++++++++-------------- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/Include/cobject.h b/Include/cobject.h index 499dfad..4e24d7d 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 5372f0b..5c9367b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,10 @@ What's New in Python 3.0 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 #3611: An exception __context__ could be cleared in a complex pattern involving a __del__ method re-raising an exception. diff --git a/Objects/cobject.c b/Objects/cobject.c index a1ee686..c437491 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 13c3f4b..9b1207f 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; } @@ -807,6 +818,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 **); @@ -856,6 +872,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, if (getbuffer(arg, (Py_buffer*)p, &buf) < 0) return converterr(buf, arg, msgbuf, bufsize); format++; + if (addcleanup(p, freelist, cleanup_buffer)) { + return converterr( + "(cleanup problem)", + arg, msgbuf, bufsize); + } break; } count = convertbuffer(arg, p, &buf); @@ -889,6 +910,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 **); @@ -1094,7 +1120,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)", @@ -1136,7 +1162,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); @@ -1249,6 +1275,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; -- cgit v0.12