From 28288bebaf61269d1e26bb0795c2de6b481e1cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Wirtel?= Date: Sat, 10 Jun 2017 14:36:57 +0200 Subject: [PATCH] bpo-30614: testInitNonExistentFile() of test_bz2 leaks references (#2033) * bpo-30614: testInitNonExistentFile() of test_bz2 leaks references Extract the code of BZ2File_dealloc and create a new BZ2File_clear() function. Call BZ2File_clear() in BZ2File_dealloc(). Define BZ2File_clear() as tp_clear. Move the lock initialization before the "self->file = PyObject_CallFunction" in BZ2File_init() and check the lock is not created twice. Call BZ2File_clear() in BZ2File_init() after the init of the lock Co-Authored-By: Victor Stinner * Create bz2module.c Fix after the review of Victor Stinner --- Modules/bz2module.c | 64 +++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/Modules/bz2module.c b/Modules/bz2module.c index fe417c5ffa0..81f168827b1 100644 --- a/Modules/bz2module.c +++ b/Modules/bz2module.c @@ -1353,6 +1353,30 @@ static PyMemberDef BZ2File_members[] = { /* ===================================================================== */ /* Slot definitions for BZ2File_Type. */ +static int +BZ2File_clear(BZ2FileObject *self) +{ + int bzerror; + + ACQUIRE_LOCK(self); + switch (self->mode) { + case MODE_READ: + case MODE_READ_EOF: + BZ2_bzReadClose(&bzerror, self->fp); + break; + case MODE_WRITE: + BZ2_bzWriteClose(&bzerror, self->fp, + 0, NULL, NULL); + break; + } + if (self->fp != NULL && self->file != NULL) + PyFile_DecUseCount((PyFileObject *)self->file); + self->fp = NULL; + Util_DropReadAhead(self); + Py_CLEAR(self->file); + RELEASE_LOCK(self); + return 0; +} static int BZ2File_init(BZ2FileObject *self, PyObject *args, PyObject *kwargs) @@ -1420,6 +1444,19 @@ BZ2File_init(BZ2FileObject *self, PyObject *args, PyObject *kwargs) mode = (mode_char == 'r') ? "rb" : "wb"; +#ifdef WITH_THREAD + if (!self->lock) { + self->lock = PyThread_allocate_lock(); + } + + if (!self->lock) { + PyErr_SetString(PyExc_MemoryError, "unable to allocate lock"); + goto error; + } +#endif + + BZ2File_clear(self); + self->file = PyObject_CallFunction((PyObject*)&PyFile_Type, "(Osi)", name, mode, buffering); if (self->file == NULL) @@ -1428,14 +1465,6 @@ BZ2File_init(BZ2FileObject *self, PyObject *args, PyObject *kwargs) /* From now on, we have stuff to dealloc, so jump to error label * instead of returning */ -#ifdef WITH_THREAD - self->lock = PyThread_allocate_lock(); - if (!self->lock) { - PyErr_SetString(PyExc_MemoryError, "unable to allocate lock"); - goto error; - } -#endif - if (mode_char == 'r') self->fp = BZ2_bzReadOpen(&bzerror, PyFile_AsFile(self->file), @@ -1469,26 +1498,11 @@ error: static void BZ2File_dealloc(BZ2FileObject *self) { - int bzerror; + BZ2File_clear(self); #ifdef WITH_THREAD if (self->lock) PyThread_free_lock(self->lock); #endif - switch (self->mode) { - case MODE_READ: - case MODE_READ_EOF: - BZ2_bzReadClose(&bzerror, self->fp); - break; - case MODE_WRITE: - BZ2_bzWriteClose(&bzerror, self->fp, - 0, NULL, NULL); - break; - } - if (self->fp != NULL && self->file != NULL) - PyFile_DecUseCount((PyFileObject *)self->file); - self->fp = NULL; - Util_DropReadAhead(self); - Py_XDECREF(self->file); Py_TYPE(self)->tp_free((PyObject *)self); } @@ -1574,7 +1588,7 @@ static PyTypeObject BZ2File_Type = { Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, /*tp_flags*/ BZ2File__doc__, /*tp_doc*/ 0, /*tp_traverse*/ - 0, /*tp_clear*/ + (inquiry)BZ2File_clear, /*tp_clear*/ 0, /*tp_richcompare*/ 0, /*tp_weaklistoffset*/ (getiterfunc)BZ2File_getiter, /*tp_iter*/