commit 28288bebaf61269d1e26bb0795c2de6b481e1cf0
author Stéphane Wirtel <stephane@wirtel.be> Sat Jun 10 14:36:57 2017 +0200
committer Victor Stinner <victor.stinner@gmail.com> Sat Jun 10 14:36:57 2017 +0200
tree d7ad6c9e2c4ae3c7945e5503bd821c562d8706d7
parent 3c7b67bb34bc537b6c0dd9c6e1e4a07dc227b467

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 <victor.stinner@gmail.com>

* Create bz2module.c

Fix after the review of Victor Stinner

Modules/bz2module.c

diff --git a/Modules/bz2module.c b/Modules/bz2module.c
index fe417c5..81f1688 100644
--- a/Modules/bz2module.c
+++ b/Modules/bz2module.c
@@ -1353,6 +1353,30 @@
 
 /* ===================================================================== */
 /* 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 @@
 
     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 @@
     /* 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 @@
 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 @@
     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*/