From 0fa02173d74cf3cc100e976240052b4d59ba45c3 Mon Sep 17 00:00:00 2001 From: Fidget-Spinner <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 27 May 2021 00:01:52 +0800 Subject: [PATCH 1/6] tp_traverse for _winapi.Overlapped --- Modules/_winapi.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index bc2126c8e2ee99..511bb8ddb238da 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -112,12 +112,22 @@ typedef struct { Py_buffer write_buffer; } OverlappedObject; +static int +overlapped_traverse(OverlappedObject *self, visitproc visit, void *arg) +{ + Py_VISIT(self->read_buffer); + Py_VISIT(self->write_buffer.obj); + Py_VISIT(Py_TYPE(self)); + return 0; +} + static void overlapped_dealloc(OverlappedObject *self) { DWORD bytes; int err = GetLastError(); + PyObject_GC_UnTrack(self); if (self->pending) { if (check_CancelIoEx() && Py_CancelIoEx(self->handle, &self->overlapped) && @@ -321,6 +331,7 @@ static PyMemberDef overlapped_members[] = { }; static PyType_Slot winapi_overlapped_type_slots[] = { + {Py_tp_traverse, overlapped_traverse}, {Py_tp_dealloc, overlapped_dealloc}, {Py_tp_doc, "OVERLAPPED structure wrapper"}, {Py_tp_methods, overlapped_methods}, @@ -331,7 +342,8 @@ static PyType_Slot winapi_overlapped_type_slots[] = { static PyType_Spec winapi_overlapped_type_spec = { .name = "_winapi.Overlapped", .basicsize = sizeof(OverlappedObject), - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION, + .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_DISALLOW_INSTANTIATION | + Py_TPFLAGS_HAVE_GC), .slots = winapi_overlapped_type_slots, }; @@ -339,7 +351,7 @@ static OverlappedObject * new_overlapped(PyObject *module, HANDLE handle) { WinApiState *st = winapi_get_state(module); - OverlappedObject *self = PyObject_New(OverlappedObject, st->overlapped_type); + OverlappedObject *self = PyObject_GC_New(OverlappedObject, st->overlapped_type); if (!self) return NULL; From 9f3ca365d75330a72bfe0404893b675937f464be Mon Sep 17 00:00:00 2001 From: Fidget-Spinner <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 27 May 2021 00:18:26 +0800 Subject: [PATCH 2/6] GC track the object --- Modules/_winapi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 511bb8ddb238da..98173430d9a459 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -363,6 +363,8 @@ new_overlapped(PyObject *module, HANDLE handle) memset(&self->write_buffer, 0, sizeof(Py_buffer)); /* Manual reset, initially non-signalled */ self->overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); + + PyObject_GC_Track(self); return self; } From 96f0d0e13447528db27f72f7f67cce0d75bf7a2a Mon Sep 17 00:00:00 2001 From: Fidget-Spinner <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 27 May 2021 00:54:14 +0800 Subject: [PATCH 3/6] move untrack down to a safer place --- Modules/_winapi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 98173430d9a459..643f6f4738958a 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -127,7 +127,6 @@ overlapped_dealloc(OverlappedObject *self) DWORD bytes; int err = GetLastError(); - PyObject_GC_UnTrack(self); if (self->pending) { if (check_CancelIoEx() && Py_CancelIoEx(self->handle, &self->overlapped) && @@ -160,6 +159,7 @@ overlapped_dealloc(OverlappedObject *self) CloseHandle(self->overlapped.hEvent); SetLastError(err); + PyObject_GC_UnTrack(self); if (self->write_buffer.obj) PyBuffer_Release(&self->write_buffer); Py_CLEAR(self->read_buffer); From 0493de09c9065c8281a7c265244b51580d65acf2 Mon Sep 17 00:00:00 2001 From: Fidget-Spinner <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 27 May 2021 01:18:08 +0800 Subject: [PATCH 4/6] add module m_traverse, clear, free - fully implement the GC protocol for real this time ;-) --- Modules/_winapi.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 643f6f4738958a..2fac7a6fb75204 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -2057,12 +2057,41 @@ static PyModuleDef_Slot winapi_slots[] = { {0, NULL} }; +static int +winapi_traverse(PyObject *m, visitproc visit, void *arg) +{ + WinApiState *st = winapi_get_state(m); + + Py_VISIT(st->overlapped_type); + + return 0; +} + +static int +winapi_clear(PyObject *m) +{ + WinApiState *st = winapi_get_state(m); + + Py_CLEAR(st->overlapped_type); + + return 0; +} + +static int +winapi_free(void *m) +{ + winapi_clear((PyObject *)m); +} + static struct PyModuleDef winapi_module = { PyModuleDef_HEAD_INIT, .m_name = "_winapi", .m_size = sizeof(WinApiState), .m_methods = winapi_functions, .m_slots = winapi_slots, + .m_traverse = winapi_traverse, + .m_clear = winapi_clear, + .m_free = winapi_free, }; PyMODINIT_FUNC From fc3e774bd7c7eea498f3629b11c80858f3ce8b7d Mon Sep 17 00:00:00 2001 From: Fidget-Spinner <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 27 May 2021 01:35:29 +0800 Subject: [PATCH 5/6] static int -> static void --- Modules/_winapi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 2fac7a6fb75204..8870ad78267277 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -2077,7 +2077,7 @@ winapi_clear(PyObject *m) return 0; } -static int +static void winapi_free(void *m) { winapi_clear((PyObject *)m); From 78113ccdb6e96dac11757823a30bf7bf72076acb Mon Sep 17 00:00:00 2001 From: Fidget-Spinner <28750310+Fidget-Spinner@users.noreply.github.com> Date: Fri, 28 May 2021 00:32:19 +0800 Subject: [PATCH 6/6] Apply victor's suggestions --- Modules/_winapi.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 8870ad78267277..30fcb4e1768eef 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -112,6 +112,11 @@ typedef struct { Py_buffer write_buffer; } OverlappedObject; +/* +Note: tp_clear (overlapped_clear) is not implemented because it +requires cancelling the IO operation if it's pending and the cancellation is +quite complex and can fail (see: overlapped_dealloc). +*/ static int overlapped_traverse(OverlappedObject *self, visitproc visit, void *arg) { @@ -2058,29 +2063,25 @@ static PyModuleDef_Slot winapi_slots[] = { }; static int -winapi_traverse(PyObject *m, visitproc visit, void *arg) +winapi_traverse(PyObject *module, visitproc visit, void *arg) { - WinApiState *st = winapi_get_state(m); - + WinApiState *st = winapi_get_state(module); Py_VISIT(st->overlapped_type); - return 0; } static int -winapi_clear(PyObject *m) +winapi_clear(PyObject *module) { - WinApiState *st = winapi_get_state(m); - + WinApiState *st = winapi_get_state(module); Py_CLEAR(st->overlapped_type); - return 0; } static void -winapi_free(void *m) +winapi_free(void *module) { - winapi_clear((PyObject *)m); + winapi_clear((PyObject *)module); } static struct PyModuleDef winapi_module = {