From 67c23500c1131048737efacf7f2aec4a9ffd13ed Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 6 Jul 2020 20:42:02 -0500 Subject: [PATCH 1/9] port winapi to multi-stage init --- Modules/_winapi.c | 129 ++++++++++++++++++++++------------------------ 1 file changed, 63 insertions(+), 66 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index ddb11aa5a82042..9e734fdff79a99 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -35,8 +35,10 @@ /* See http://www.python.org/2.4/license for licensing details. */ #include "Python.h" +#include "moduleobject.h" // PyModuleDef_Slot #include "structmember.h" // PyMemberDef + #define WINDOWS_LEAN_AND_MEAN #include "windows.h" #include @@ -78,6 +80,17 @@ check_CancelIoEx() return has_CancelIoEx; } +typedef struct { + PyTypeObject *overlapped_type; +} WinApiState; + +static inline WinApiState* +winapi_get_state(PyObject *module) +{ + void *state = PyModule_GetState(module); + assert(state != NULL); + return (WinApiState *)state; +} /* * A Python object wrapping an OVERLAPPED structure and other useful data @@ -305,53 +318,30 @@ static PyMemberDef overlapped_members[] = { {NULL} }; -PyTypeObject OverlappedType = { - PyVarObject_HEAD_INIT(NULL, 0) - /* tp_name */ "_winapi.Overlapped", - /* tp_basicsize */ sizeof(OverlappedObject), - /* tp_itemsize */ 0, - /* tp_dealloc */ (destructor) overlapped_dealloc, - /* tp_vectorcall_offset */ 0, - /* tp_getattr */ 0, - /* tp_setattr */ 0, - /* tp_as_async */ 0, - /* tp_repr */ 0, - /* tp_as_number */ 0, - /* tp_as_sequence */ 0, - /* tp_as_mapping */ 0, - /* tp_hash */ 0, - /* tp_call */ 0, - /* tp_str */ 0, - /* tp_getattro */ 0, - /* tp_setattro */ 0, - /* tp_as_buffer */ 0, - /* tp_flags */ Py_TPFLAGS_DEFAULT, - /* tp_doc */ "OVERLAPPED structure wrapper", - /* tp_traverse */ 0, - /* tp_clear */ 0, - /* tp_richcompare */ 0, - /* tp_weaklistoffset */ 0, - /* tp_iter */ 0, - /* tp_iternext */ 0, - /* tp_methods */ overlapped_methods, - /* tp_members */ overlapped_members, - /* tp_getset */ 0, - /* tp_base */ 0, - /* tp_dict */ 0, - /* tp_descr_get */ 0, - /* tp_descr_set */ 0, - /* tp_dictoffset */ 0, - /* tp_init */ 0, - /* tp_alloc */ 0, - /* tp_new */ 0, +static PyType_Slot winapi_overlapped_type_slots[] = { + {Py_tp_dealloc, overlapped_dealloc}, + {Py_tp_doc, "OVERLAPPED structure wrapper"}, + {Py_tp_methods, overlapped_methods}, + {Py_tp_members, overlapped_members}, + {0,0} +}; + +static PyType_Spec winapi_overlapped_type_spec = { + .name = "_winapi.Overlapped", + .basicsize = sizeof(OverlappedObject), + .flags = Py_TPFLAGS_DEFAULT, + .slots = winapi_overlapped_type_slots, }; static OverlappedObject * -new_overlapped(HANDLE handle) +new_overlapped(PyObject *module, HANDLE handle) { OverlappedObject *self; - self = PyObject_New(OverlappedObject, &OverlappedType); + WinApiState *st; + st = winapi_get_state(module); + + self = PyObject_New(OverlappedObject, st->overlapped_type); if (!self) return NULL; self->handle = handle; @@ -409,7 +399,7 @@ _winapi_ConnectNamedPipe_impl(PyObject *module, HANDLE handle, OverlappedObject *overlapped = NULL; if (use_overlapped) { - overlapped = new_overlapped(handle); + overlapped = new_overlapped(module, handle); if (!overlapped) return NULL; } @@ -1527,7 +1517,7 @@ _winapi_ReadFile_impl(PyObject *module, HANDLE handle, DWORD size, if (!buf) return NULL; if (use_overlapped) { - overlapped = new_overlapped(handle); + overlapped = new_overlapped(module, handle); if (!overlapped) { Py_DECREF(buf); return NULL; @@ -1810,7 +1800,7 @@ _winapi_WriteFile_impl(PyObject *module, HANDLE handle, PyObject *buffer, OverlappedObject *overlapped = NULL; if (use_overlapped) { - overlapped = new_overlapped(handle); + overlapped = new_overlapped(module, handle); if (!overlapped) return NULL; buf = &overlapped->write_buffer; @@ -1921,36 +1911,24 @@ static PyMethodDef winapi_functions[] = { {NULL, NULL} }; -static struct PyModuleDef winapi_module = { - PyModuleDef_HEAD_INIT, - "_winapi", - NULL, - -1, - winapi_functions, - NULL, - NULL, - NULL, - NULL -}; - #define WINAPI_CONSTANT(fmt, con) \ PyDict_SetItemString(d, #con, Py_BuildValue(fmt, con)) -PyMODINIT_FUNC -PyInit__winapi(void) +static int winapi_exec(PyObject *m) { PyObject *d; - PyObject *m; + WinApiState *st; - if (PyType_Ready(&OverlappedType) < 0) - return NULL; + st = winapi_get_state(m); + + st->overlapped_type = (PyTypeObject *)PyType_FromModuleAndSpec(m, &winapi_overlapped_type_spec, NULL); + if (st->overlapped_type == NULL) { + return -1; + } - m = PyModule_Create(&winapi_module); - if (m == NULL) - return NULL; d = PyModule_GetDict(m); - PyDict_SetItemString(d, "Overlapped", (PyObject *) &OverlappedType); + PyDict_SetItemString(d, "Overlapped", (PyObject *) st->overlapped_type); /* constants */ WINAPI_CONSTANT(F_DWORD, CREATE_NEW_CONSOLE); @@ -2049,5 +2027,24 @@ PyInit__winapi(void) WINAPI_CONSTANT("i", NULL); - return m; + return 0; +} + +static PyModuleDef_Slot winapi_slots[] = { + {Py_mod_exec, winapi_exec}, + {0, NULL} +}; + +static struct PyModuleDef winapi_module = { + PyModuleDef_HEAD_INIT, + .m_name = "_winapi", + .m_size = sizeof(WinApiState), + .m_methods = winapi_functions, + .m_slots = winapi_slots, +}; + +PyMODINIT_FUNC +PyInit__winapi(void) +{ + return PyModuleDef_Init(&winapi_module); } From 12c41c815e95e218bc2b8274c389a3bbc000cdca Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 6 Jul 2020 20:43:31 -0500 Subject: [PATCH 2/9] blurb --- .../Core and Builtins/2020-07-06-20-43-19.bpo-1635741.LYhsni.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-07-06-20-43-19.bpo-1635741.LYhsni.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-07-06-20-43-19.bpo-1635741.LYhsni.rst b/Misc/NEWS.d/next/Core and Builtins/2020-07-06-20-43-19.bpo-1635741.LYhsni.rst new file mode 100644 index 00000000000000..956fcd5d1ee29e --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-07-06-20-43-19.bpo-1635741.LYhsni.rst @@ -0,0 +1 @@ +Port :mod:`winapi` to multiphase initialization From f5e2fc29d1afe71106e930fb9bf0e8e9fb00d589 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Tue, 7 Jul 2020 13:11:28 -0500 Subject: [PATCH 3/9] reviewer suggestions --- Modules/_winapi.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 9e734fdff79a99..540e8ca0433d7d 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -338,12 +338,11 @@ new_overlapped(PyObject *module, HANDLE handle) { OverlappedObject *self; - WinApiState *st; - st = winapi_get_state(module); - + WinApiState *st = winapi_get_state(module); self = PyObject_New(OverlappedObject, st->overlapped_type); if (!self) return NULL; + self->handle = handle; self->read_buffer = NULL; self->pending = 0; @@ -1912,23 +1911,23 @@ static PyMethodDef winapi_functions[] = { }; #define WINAPI_CONSTANT(fmt, con) \ - PyDict_SetItemString(d, #con, Py_BuildValue(fmt, con)) + if (PyDict_SetItemString(d, #con, Py_BuildValue(fmt, con)) < 0) return -1 static int winapi_exec(PyObject *m) { - PyObject *d; - WinApiState *st; - - st = winapi_get_state(m); + WinApiState *st = winapi_get_state(m); st->overlapped_type = (PyTypeObject *)PyType_FromModuleAndSpec(m, &winapi_overlapped_type_spec, NULL); if (st->overlapped_type == NULL) { return -1; } - d = PyModule_GetDict(m); + if (PyModule_AddObject(m, "Overlapped", (PyObject *)st->overlapped_type) < 0) { + Py_DECREF(st->overlapped_type); + return -1; + } - PyDict_SetItemString(d, "Overlapped", (PyObject *) st->overlapped_type); + PyObject *d = PyModule_GetDict(m); /* constants */ WINAPI_CONSTANT(F_DWORD, CREATE_NEW_CONSOLE); From 283911c92cf8c72e761347d652d32f852080dee1 Mon Sep 17 00:00:00 2001 From: = <=> Date: Tue, 7 Jul 2020 21:25:05 -0500 Subject: [PATCH 4/9] fix refcount --- Modules/_winapi.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 540e8ca0433d7d..49269255693a7b 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1910,8 +1910,20 @@ static PyMethodDef winapi_functions[] = { {NULL, NULL} }; +static int +SetWinAPIConstant(PyObject* d, const char* fmt, const char* constantString, int constant) { + PyObject *value = Py_BuildValue(fmt, constant); + if (PyDict_SetItemString(d, constantString, value) < 0) { + Py_DECREF(value); + return -1; + } + + Py_DECREF(value); + return 0; +} + #define WINAPI_CONSTANT(fmt, con) \ - if (PyDict_SetItemString(d, #con, Py_BuildValue(fmt, con)) < 0) return -1 + if (SetWinAPIConstant(d, fmt, #con, con) < 0) return -1 static int winapi_exec(PyObject *m) { From e0e1b30cf0812876d7ebba93de3468fe3b99c6b5 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Thu, 9 Jul 2020 01:33:36 -0500 Subject: [PATCH 5/9] update based on review --- Modules/_winapi.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 49269255693a7b..81fd3bab55fe8b 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1910,21 +1910,15 @@ static PyMethodDef winapi_functions[] = { {NULL, NULL} }; -static int -SetWinAPIConstant(PyObject* d, const char* fmt, const char* constantString, int constant) { - PyObject *value = Py_BuildValue(fmt, constant); - if (PyDict_SetItemString(d, constantString, value) < 0) { - Py_DECREF(value); - return -1; - } - - Py_DECREF(value); - return 0; +#define WINAPI_CONSTANT(fmt, con) { \ + PyObject *value = Py_BuildValue(fmt, con); \ + if (PyDict_SetItemString(d, #con, value) < 0) { \ + Py_DECREF(value); \ + return -1; \ + } \ + Py_DECREF(value); \ } -#define WINAPI_CONSTANT(fmt, con) \ - if (SetWinAPIConstant(d, fmt, #con, con) < 0) return -1 - static int winapi_exec(PyObject *m) { WinApiState *st = winapi_get_state(m); @@ -1934,6 +1928,7 @@ static int winapi_exec(PyObject *m) return -1; } + Py_INCREF(st->overlapped_type); if (PyModule_AddObject(m, "Overlapped", (PyObject *)st->overlapped_type) < 0) { Py_DECREF(st->overlapped_type); return -1; From 71957436e8b88533ce9d7ed9402d68178d7c9798 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Thu, 9 Jul 2020 01:51:22 -0500 Subject: [PATCH 6/9] update based on review --- Modules/_winapi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 81fd3bab55fe8b..6199dcb6af915a 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1929,7 +1929,7 @@ static int winapi_exec(PyObject *m) } Py_INCREF(st->overlapped_type); - if (PyModule_AddObject(m, "Overlapped", (PyObject *)st->overlapped_type) < 0) { + if (PyModule_AddType(m, st->overlapped_type) < 0) { Py_DECREF(st->overlapped_type); return -1; } From a836c71faf6658d83d4d9bd3ea9c00ae5c04556a Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Thu, 9 Jul 2020 09:27:56 -0500 Subject: [PATCH 7/9] update based on review --- Modules/_winapi.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 6199dcb6af915a..3b49f4e953724e 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -153,7 +153,9 @@ overlapped_dealloc(OverlappedObject *self) if (self->write_buffer.obj) PyBuffer_Release(&self->write_buffer); Py_CLEAR(self->read_buffer); - PyObject_Del(self); + PyTypeObject *tp = PY_TYPE(self); + tp->tp_free(self); + Py_DECREF(tp); } /*[clinic input] @@ -336,10 +338,8 @@ static PyType_Spec winapi_overlapped_type_spec = { static OverlappedObject * new_overlapped(PyObject *module, HANDLE handle) { - OverlappedObject *self; - WinApiState *st = winapi_get_state(module); - self = PyObject_New(OverlappedObject, st->overlapped_type); + OverlappedObject *self = PyObject_New(OverlappedObject, st->overlapped_type); if (!self) return NULL; @@ -1910,14 +1910,15 @@ static PyMethodDef winapi_functions[] = { {NULL, NULL} }; -#define WINAPI_CONSTANT(fmt, con) { \ - PyObject *value = Py_BuildValue(fmt, con); \ - if (PyDict_SetItemString(d, #con, value) < 0) { \ +#define WINAPI_CONSTANT(fmt, con) \ + do { \ + PyObject *value = Py_BuildValue(fmt, con); \ + if (PyDict_SetItemString(d, #con, value) < 0) { \ + Py_DECREF(value); \ + return -1; \ + } \ Py_DECREF(value); \ - return -1; \ - } \ - Py_DECREF(value); \ -} + } while (0) static int winapi_exec(PyObject *m) { @@ -1928,9 +1929,7 @@ static int winapi_exec(PyObject *m) return -1; } - Py_INCREF(st->overlapped_type); if (PyModule_AddType(m, st->overlapped_type) < 0) { - Py_DECREF(st->overlapped_type); return -1; } From af115c1602c85d94f10f134a5868fa4869471659 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Thu, 9 Jul 2020 09:41:46 -0500 Subject: [PATCH 8/9] fix typo --- Modules/_winapi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 3b49f4e953724e..927e97a401bb81 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -153,7 +153,7 @@ overlapped_dealloc(OverlappedObject *self) if (self->write_buffer.obj) PyBuffer_Release(&self->write_buffer); Py_CLEAR(self->read_buffer); - PyTypeObject *tp = PY_TYPE(self); + PyTypeObject *tp = Py_TYPE(self); tp->tp_free(self); Py_DECREF(tp); } From 42352a4f991ef74e299a38fd24a5a4d50189b3f3 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 10 Aug 2020 09:06:07 -0500 Subject: [PATCH 9/9] check result of Py_BuildValue --- Modules/_winapi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 927e97a401bb81..7ba14095c96e19 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1913,6 +1913,9 @@ static PyMethodDef winapi_functions[] = { #define WINAPI_CONSTANT(fmt, con) \ do { \ PyObject *value = Py_BuildValue(fmt, con); \ + if (value == NULL) { \ + return -1; \ + } \ if (PyDict_SetItemString(d, #con, value) < 0) { \ Py_DECREF(value); \ return -1; \