From 5485085b324a45307c1ff4ec7d85b5998d7d5e0d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 11 Jan 2019 14:35:14 +0100 Subject: [PATCH] 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. --- .../2019-01-10-15-55-10.bpo-32710.KwECPu.rst | 2 + Modules/overlapped.c | 78 +++++++++++++------ 2 files changed, 56 insertions(+), 24 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-01-10-15-55-10.bpo-32710.KwECPu.rst 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 index 00000000000..9f7a95a0aaf --- /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 bbaa4fb3008..ef4390b8254 100644 --- 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,