bpo-32710: Fix _overlapped.Overlapped memory leaks (GH-11489)

Fix memory leaks in asyncio ProactorEventLoop on overlapped operation
failures.

Changes:

* Implement the tp_traverse slot in the _overlapped.Overlapped type
  to help to break reference cycles and identify referrers in the
  garbage collector.
* Always clear overlapped on failure: not only set type to
  TYPE_NOT_STARTED, but release also resources.

diff --git a/Misc/NEWS.d/next/Library/2019-01-10-15-55-10.bpo-32710.KwECPu.rst b/Misc/NEWS.d/next/Library/2019-01-10-15-55-10.bpo-32710.KwECPu.rst
new file mode 100644 (file)
index 0000000..9f7a95a
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-01-10-15-55-10.bpo-32710.KwECPu.rst
@@ -0,0 +1,2 @@
+Fix memory leaks in asyncio ProactorEventLoop on overlapped operation
+failure.

diff --git a/Modules/overlapped.c b/Modules/overlapped.c
index bbaa4fb3008f0979ed1d26b17ad075598946fe91..ef4390b82548be1195faed2bd40da362c4193c17 100644 (file)
--- a/Modules/overlapped.c
+++ b/Modules/overlapped.c
@@ -561,6 +561,28 @@ Overlapped_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
     return (PyObject *)self;
 }
 
+
+/* Note (bpo-32710): OverlappedType.tp_clear is not defined to not release
+   buffers while overlapped are still running, to prevent a crash. */
+static int
+Overlapped_clear(OverlappedObject *self)
+{
+    switch (self->type) {
+    case TYPE_READ:
+    case TYPE_ACCEPT:
+        Py_CLEAR(self->allocated_buffer);
+        break;
+    case TYPE_WRITE:
+    case TYPE_READINTO:
+        if (self->user_buffer.obj) {
+            PyBuffer_Release(&self->user_buffer);
+        }
+        break;
+    }
+    self->type = TYPE_NOT_STARTED;
+    return 0;
+}
+
 static void
 Overlapped_dealloc(OverlappedObject *self)
 {
@@ -594,20 +616,11 @@ Overlapped_dealloc(OverlappedObject *self)
         }
     }
 
-    if (self->overlapped.hEvent != NULL)
+    if (self->overlapped.hEvent != NULL) {
         CloseHandle(self->overlapped.hEvent);
-
-    switch (self->type) {
-    case TYPE_READ:
-    case TYPE_ACCEPT:
-        Py_CLEAR(self->allocated_buffer);
-        break;
-    case TYPE_WRITE:
-    case TYPE_READINTO:
-        if (self->user_buffer.obj)
-            PyBuffer_Release(&self->user_buffer);
-        break;
     }
+
+    Overlapped_clear(self);
     PyObject_Del(self);
     SetLastError(olderr);
 }
@@ -723,8 +736,7 @@ do_ReadFile(OverlappedObject *self, HANDLE handle,
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            PyBuffer_Release(&self->user_buffer);
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -827,7 +839,7 @@ do_WSARecv(OverlappedObject *self, HANDLE handle,
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -955,7 +967,7 @@ Overlapped_WriteFile(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1012,8 +1024,7 @@ Overlapped_WSASend(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            PyBuffer_Release(&self->user_buffer);
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1063,7 +1074,7 @@ Overlapped_AcceptEx(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1155,7 +1166,7 @@ Overlapped_ConnectEx(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1194,7 +1205,7 @@ Overlapped_DisconnectEx(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1249,7 +1260,7 @@ Overlapped_TransmitFile(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_NONE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1290,7 +1301,7 @@ Overlapped_ConnectNamedPipe(OverlappedObject *self, PyObject *args)
         case ERROR_IO_PENDING:
             Py_RETURN_FALSE;
         default:
-            self->type = TYPE_NOT_STARTED;
+            Overlapped_clear(self);
             return SetFromWindowsErr(err);
     }
 }
@@ -1340,6 +1351,25 @@ Overlapped_getpending(OverlappedObject *self)
                            self->type != TYPE_NOT_STARTED);
 }
 
+static int
+Overlapped_traverse(OverlappedObject *self, visitproc visit, void *arg)
+{
+    switch (self->type) {
+    case TYPE_READ:
+    case TYPE_ACCEPT:
+        Py_VISIT(self->allocated_buffer);
+        break;
+    case TYPE_WRITE:
+    case TYPE_READINTO:
+        if (self->user_buffer.obj) {
+            Py_VISIT(&self->user_buffer.obj);
+        }
+        break;
+    }
+    return 0;
+}
+
+
 static PyMethodDef Overlapped_methods[] = {
     {"getresult", (PyCFunction) Overlapped_getresult,
      METH_VARARGS, Overlapped_getresult_doc},
@@ -1410,7 +1440,7 @@ PyTypeObject OverlappedType = {
     /* tp_as_buffer      */ 0,
     /* tp_flags          */ Py_TPFLAGS_DEFAULT,
     /* tp_doc            */ "OVERLAPPED structure wrapper",
-    /* tp_traverse       */ 0,
+    /* tp_traverse       */ (traverseproc)Overlapped_traverse,
     /* tp_clear          */ 0,
     /* tp_richcompare    */ 0,
     /* tp_weaklistoffset */ 0,