From 7d9f2fd4f28b6d8d60b7af6b34b50a767bd223a9 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 28 Jan 2024 10:56:03 +0300 Subject: [PATCH 1/7] gh-114670: Fix `_testbuffer` module initialization --- Modules/_testbuffer.c | 107 +++++++++++++++++++++++++----------------- 1 file changed, 65 insertions(+), 42 deletions(-) diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c index 5101834cfe1387..6590b8da91bcda 100644 --- a/Modules/_testbuffer.c +++ b/Modules/_testbuffer.c @@ -2823,63 +2823,86 @@ PyInit__testbuffer(void) PyObject *m; m = PyModule_Create(&_testbuffermodule); - if (m == NULL) + if (m == NULL) { return NULL; + } Py_SET_TYPE(&NDArray_Type, &PyType_Type); Py_INCREF(&NDArray_Type); - PyModule_AddObject(m, "ndarray", (PyObject *)&NDArray_Type); + if (PyModule_AddObject(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { + Py_DECREF(&NDArray_Type); + return NULL; + } Py_SET_TYPE(&StaticArray_Type, &PyType_Type); Py_INCREF(&StaticArray_Type); - PyModule_AddObject(m, "staticarray", (PyObject *)&StaticArray_Type); + if (PyModule_AddObject(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { + goto error; + } structmodule = PyImport_ImportModule("struct"); - if (structmodule == NULL) - return NULL; + if (structmodule == NULL) { + goto error; + } Struct = PyObject_GetAttrString(structmodule, "Struct"); + if (Struct == NULL) { + goto error; + } calcsize = PyObject_GetAttrString(structmodule, "calcsize"); - if (Struct == NULL || calcsize == NULL) - return NULL; + if (calcsize == NULL) { + goto error; + } simple_format = PyUnicode_FromString(simple_fmt); - if (simple_format == NULL) - return NULL; + if (simple_format == NULL) { + goto error; + } - PyModule_AddIntMacro(m, ND_MAX_NDIM); - PyModule_AddIntMacro(m, ND_VAREXPORT); - PyModule_AddIntMacro(m, ND_WRITABLE); - PyModule_AddIntMacro(m, ND_FORTRAN); - PyModule_AddIntMacro(m, ND_SCALAR); - PyModule_AddIntMacro(m, ND_PIL); - PyModule_AddIntMacro(m, ND_GETBUF_FAIL); - PyModule_AddIntMacro(m, ND_GETBUF_UNDEFINED); - PyModule_AddIntMacro(m, ND_REDIRECT); - - PyModule_AddIntMacro(m, PyBUF_SIMPLE); - PyModule_AddIntMacro(m, PyBUF_WRITABLE); - PyModule_AddIntMacro(m, PyBUF_FORMAT); - PyModule_AddIntMacro(m, PyBUF_ND); - PyModule_AddIntMacro(m, PyBUF_STRIDES); - PyModule_AddIntMacro(m, PyBUF_INDIRECT); - PyModule_AddIntMacro(m, PyBUF_C_CONTIGUOUS); - PyModule_AddIntMacro(m, PyBUF_F_CONTIGUOUS); - PyModule_AddIntMacro(m, PyBUF_ANY_CONTIGUOUS); - PyModule_AddIntMacro(m, PyBUF_FULL); - PyModule_AddIntMacro(m, PyBUF_FULL_RO); - PyModule_AddIntMacro(m, PyBUF_RECORDS); - PyModule_AddIntMacro(m, PyBUF_RECORDS_RO); - PyModule_AddIntMacro(m, PyBUF_STRIDED); - PyModule_AddIntMacro(m, PyBUF_STRIDED_RO); - PyModule_AddIntMacro(m, PyBUF_CONTIG); - PyModule_AddIntMacro(m, PyBUF_CONTIG_RO); - - PyModule_AddIntMacro(m, PyBUF_READ); - PyModule_AddIntMacro(m, PyBUF_WRITE); +#define ADD_INT_MACRO(m, macro) \ + do { \ + if (PyModule_AddIntConstant(m, #macro, macro) < 0) { \ + goto error; \ + } \ + } while (0) + + ADD_INT_MACRO(m, ND_MAX_NDIM); + ADD_INT_MACRO(m, ND_VAREXPORT); + ADD_INT_MACRO(m, ND_WRITABLE); + ADD_INT_MACRO(m, ND_FORTRAN); + ADD_INT_MACRO(m, ND_SCALAR); + ADD_INT_MACRO(m, ND_PIL); + ADD_INT_MACRO(m, ND_GETBUF_FAIL); + ADD_INT_MACRO(m, ND_GETBUF_UNDEFINED); + ADD_INT_MACRO(m, ND_REDIRECT); + + ADD_INT_MACRO(m, PyBUF_SIMPLE); + ADD_INT_MACRO(m, PyBUF_WRITABLE); + ADD_INT_MACRO(m, PyBUF_FORMAT); + ADD_INT_MACRO(m, PyBUF_ND); + ADD_INT_MACRO(m, PyBUF_STRIDES); + ADD_INT_MACRO(m, PyBUF_INDIRECT); + ADD_INT_MACRO(m, PyBUF_C_CONTIGUOUS); + ADD_INT_MACRO(m, PyBUF_F_CONTIGUOUS); + ADD_INT_MACRO(m, PyBUF_ANY_CONTIGUOUS); + ADD_INT_MACRO(m, PyBUF_FULL); + ADD_INT_MACRO(m, PyBUF_FULL_RO); + ADD_INT_MACRO(m, PyBUF_RECORDS); + ADD_INT_MACRO(m, PyBUF_RECORDS_RO); + ADD_INT_MACRO(m, PyBUF_STRIDED); + ADD_INT_MACRO(m, PyBUF_STRIDED_RO); + ADD_INT_MACRO(m, PyBUF_CONTIG); + ADD_INT_MACRO(m, PyBUF_CONTIG_RO); + + ADD_INT_MACRO(m, PyBUF_READ); + ADD_INT_MACRO(m, PyBUF_WRITE); + +#undef ADD_INT_MACRO return m; -} - - +error: + Py_DECREF(&NDArray_Type); + Py_DECREF(&StaticArray_Type); + return NULL; +} From 63c2bb22ef647a7cf6df7724c0e2c6510d58a0b0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 28 Jan 2024 22:32:16 +0300 Subject: [PATCH 2/7] Address review --- ...-01-28-22-32-07.gh-issue-114672.9iEHAg.rst | 1 + Modules/_testbuffer.c | 52 ++++++++++--------- 2 files changed, 28 insertions(+), 25 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst new file mode 100644 index 00000000000000..4a2bede9ffa9cf --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst @@ -0,0 +1 @@ +Fix ``_testbuffer`` module initialization. diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c index 6590b8da91bcda..29d7729337a48b 100644 --- a/Modules/_testbuffer.c +++ b/Modules/_testbuffer.c @@ -2816,53 +2816,44 @@ static struct PyModuleDef _testbuffermodule = { NULL }; - -PyMODINIT_FUNC -PyInit__testbuffer(void) +static int +_testbuffer_exec(PyObject *m) { - PyObject *m; - - m = PyModule_Create(&_testbuffermodule); - if (m == NULL) { - return NULL; - } - Py_SET_TYPE(&NDArray_Type, &PyType_Type); Py_INCREF(&NDArray_Type); - if (PyModule_AddObject(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { - Py_DECREF(&NDArray_Type); - return NULL; + if (PyModule_Add(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { + return -1; } Py_SET_TYPE(&StaticArray_Type, &PyType_Type); Py_INCREF(&StaticArray_Type); - if (PyModule_AddObject(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { - goto error; + if (PyModule_Add(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { + return -1; } structmodule = PyImport_ImportModule("struct"); if (structmodule == NULL) { - goto error; + return -1; } Struct = PyObject_GetAttrString(structmodule, "Struct"); if (Struct == NULL) { - goto error; + return -1; } calcsize = PyObject_GetAttrString(structmodule, "calcsize"); if (calcsize == NULL) { - goto error; + return -1; } simple_format = PyUnicode_FromString(simple_fmt); if (simple_format == NULL) { - goto error; + return -1; } #define ADD_INT_MACRO(m, macro) \ do { \ if (PyModule_AddIntConstant(m, #macro, macro) < 0) { \ - goto error; \ + return -1; \ } \ } while (0) @@ -2899,10 +2890,21 @@ PyInit__testbuffer(void) #undef ADD_INT_MACRO - return m; + return 0; +} -error: - Py_DECREF(&NDArray_Type); - Py_DECREF(&StaticArray_Type); - return NULL; +PyMODINIT_FUNC +PyInit__testbuffer(void) +{ + PyObject *m; + + m = PyModule_Create(&_testbuffermodule); + if (m == NULL) { + return NULL; + } + if (_testbuffer_exec(m) < 0) { + Py_DECREF(m); + return NULL; + } + return m; } From c36c054aa95c5ca8c044b585b2083b4908fd83f0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 7 Feb 2024 15:10:06 +0300 Subject: [PATCH 3/7] Do not leak `structmodule` and `simple_format` --- Modules/_testbuffer.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c index 29d7729337a48b..776b1ff66714b5 100644 --- a/Modules/_testbuffer.c +++ b/Modules/_testbuffer.c @@ -2822,38 +2822,38 @@ _testbuffer_exec(PyObject *m) Py_SET_TYPE(&NDArray_Type, &PyType_Type); Py_INCREF(&NDArray_Type); if (PyModule_Add(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { - return -1; + goto error; } Py_SET_TYPE(&StaticArray_Type, &PyType_Type); Py_INCREF(&StaticArray_Type); if (PyModule_Add(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { - return -1; + goto error; } structmodule = PyImport_ImportModule("struct"); if (structmodule == NULL) { - return -1; + goto error; } Struct = PyObject_GetAttrString(structmodule, "Struct"); if (Struct == NULL) { - return -1; + goto error; } calcsize = PyObject_GetAttrString(structmodule, "calcsize"); if (calcsize == NULL) { - return -1; + goto error; } simple_format = PyUnicode_FromString(simple_fmt); if (simple_format == NULL) { - return -1; + goto error; } #define ADD_INT_MACRO(m, macro) \ do { \ if (PyModule_AddIntConstant(m, #macro, macro) < 0) { \ - return -1; \ + goto error; \ } \ } while (0) @@ -2891,6 +2891,11 @@ _testbuffer_exec(PyObject *m) #undef ADD_INT_MACRO return 0; + +error: + Py_XDECREF(structmodule); + Py_XDECREF(simple_format); + return -1; } PyMODINIT_FUNC From 5a56dbe9961c822e8862df0f4b99bcb3bc9aac74 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Wed, 7 Feb 2024 16:00:16 +0300 Subject: [PATCH 4/7] Revert "Do not leak `structmodule` and `simple_format`" This reverts commit c36c054aa95c5ca8c044b585b2083b4908fd83f0. --- Modules/_testbuffer.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c index 776b1ff66714b5..29d7729337a48b 100644 --- a/Modules/_testbuffer.c +++ b/Modules/_testbuffer.c @@ -2822,38 +2822,38 @@ _testbuffer_exec(PyObject *m) Py_SET_TYPE(&NDArray_Type, &PyType_Type); Py_INCREF(&NDArray_Type); if (PyModule_Add(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { - goto error; + return -1; } Py_SET_TYPE(&StaticArray_Type, &PyType_Type); Py_INCREF(&StaticArray_Type); if (PyModule_Add(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { - goto error; + return -1; } structmodule = PyImport_ImportModule("struct"); if (structmodule == NULL) { - goto error; + return -1; } Struct = PyObject_GetAttrString(structmodule, "Struct"); if (Struct == NULL) { - goto error; + return -1; } calcsize = PyObject_GetAttrString(structmodule, "calcsize"); if (calcsize == NULL) { - goto error; + return -1; } simple_format = PyUnicode_FromString(simple_fmt); if (simple_format == NULL) { - goto error; + return -1; } #define ADD_INT_MACRO(m, macro) \ do { \ if (PyModule_AddIntConstant(m, #macro, macro) < 0) { \ - goto error; \ + return -1; \ } \ } while (0) @@ -2891,11 +2891,6 @@ _testbuffer_exec(PyObject *m) #undef ADD_INT_MACRO return 0; - -error: - Py_XDECREF(structmodule); - Py_XDECREF(simple_format); - return -1; } PyMODINIT_FUNC From b8ecc30396aaac9a0848f4cc2b88d3ca8ece0e78 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 8 Feb 2024 20:41:49 +0300 Subject: [PATCH 5/7] Address review --- .../2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst | 2 +- Modules/_testbuffer.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst index 4a2bede9ffa9cf..ecb9ad7ab2d8d7 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst @@ -1 +1 @@ -Fix ``_testbuffer`` module initialization. +Fix reference leaks during ``_testbuffer`` module initialization. diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c index 29d7729337a48b..d8bd474bc19d7b 100644 --- a/Modules/_testbuffer.c +++ b/Modules/_testbuffer.c @@ -2820,14 +2820,12 @@ static int _testbuffer_exec(PyObject *m) { Py_SET_TYPE(&NDArray_Type, &PyType_Type); - Py_INCREF(&NDArray_Type); - if (PyModule_Add(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { + if (PyModule_AddObjectRef(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { return -1; } Py_SET_TYPE(&StaticArray_Type, &PyType_Type); - Py_INCREF(&StaticArray_Type); - if (PyModule_Add(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { + if (PyModule_AddObjectRef(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { return -1; } From cfcfb6a8b9ff3f951dcfdecbedbcfef0de5c9396 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 8 Feb 2024 20:51:57 +0300 Subject: [PATCH 6/7] Address review --- ...-01-28-22-32-07.gh-issue-114672.9iEHAg.rst | 3 +- Modules/_testbuffer.c | 82 +++++++++---------- 2 files changed, 42 insertions(+), 43 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst index ecb9ad7ab2d8d7..3c11cf76597c18 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst @@ -1 +1,2 @@ -Fix reference leaks during ``_testbuffer`` module initialization. +Fix reference leaks during ``_testbuffer`` module initialization +in error handling. diff --git a/Modules/_testbuffer.c b/Modules/_testbuffer.c index d8bd474bc19d7b..5084bcadb10f85 100644 --- a/Modules/_testbuffer.c +++ b/Modules/_testbuffer.c @@ -2817,15 +2817,15 @@ static struct PyModuleDef _testbuffermodule = { }; static int -_testbuffer_exec(PyObject *m) +_testbuffer_exec(PyObject *mod) { Py_SET_TYPE(&NDArray_Type, &PyType_Type); - if (PyModule_AddObjectRef(m, "ndarray", (PyObject *)&NDArray_Type) < 0) { + if (PyModule_AddType(mod, &NDArray_Type) < 0) { return -1; } Py_SET_TYPE(&StaticArray_Type, &PyType_Type); - if (PyModule_AddObjectRef(m, "staticarray", (PyObject *)&StaticArray_Type) < 0) { + if (PyModule_AddType(mod, &StaticArray_Type) < 0) { return -1; } @@ -2848,43 +2848,43 @@ _testbuffer_exec(PyObject *m) return -1; } -#define ADD_INT_MACRO(m, macro) \ +#define ADD_INT_MACRO(mod, macro) \ do { \ - if (PyModule_AddIntConstant(m, #macro, macro) < 0) { \ + if (PyModule_AddIntConstant(mod, #macro, macro) < 0) { \ return -1; \ } \ } while (0) - ADD_INT_MACRO(m, ND_MAX_NDIM); - ADD_INT_MACRO(m, ND_VAREXPORT); - ADD_INT_MACRO(m, ND_WRITABLE); - ADD_INT_MACRO(m, ND_FORTRAN); - ADD_INT_MACRO(m, ND_SCALAR); - ADD_INT_MACRO(m, ND_PIL); - ADD_INT_MACRO(m, ND_GETBUF_FAIL); - ADD_INT_MACRO(m, ND_GETBUF_UNDEFINED); - ADD_INT_MACRO(m, ND_REDIRECT); - - ADD_INT_MACRO(m, PyBUF_SIMPLE); - ADD_INT_MACRO(m, PyBUF_WRITABLE); - ADD_INT_MACRO(m, PyBUF_FORMAT); - ADD_INT_MACRO(m, PyBUF_ND); - ADD_INT_MACRO(m, PyBUF_STRIDES); - ADD_INT_MACRO(m, PyBUF_INDIRECT); - ADD_INT_MACRO(m, PyBUF_C_CONTIGUOUS); - ADD_INT_MACRO(m, PyBUF_F_CONTIGUOUS); - ADD_INT_MACRO(m, PyBUF_ANY_CONTIGUOUS); - ADD_INT_MACRO(m, PyBUF_FULL); - ADD_INT_MACRO(m, PyBUF_FULL_RO); - ADD_INT_MACRO(m, PyBUF_RECORDS); - ADD_INT_MACRO(m, PyBUF_RECORDS_RO); - ADD_INT_MACRO(m, PyBUF_STRIDED); - ADD_INT_MACRO(m, PyBUF_STRIDED_RO); - ADD_INT_MACRO(m, PyBUF_CONTIG); - ADD_INT_MACRO(m, PyBUF_CONTIG_RO); - - ADD_INT_MACRO(m, PyBUF_READ); - ADD_INT_MACRO(m, PyBUF_WRITE); + ADD_INT_MACRO(mod, ND_MAX_NDIM); + ADD_INT_MACRO(mod, ND_VAREXPORT); + ADD_INT_MACRO(mod, ND_WRITABLE); + ADD_INT_MACRO(mod, ND_FORTRAN); + ADD_INT_MACRO(mod, ND_SCALAR); + ADD_INT_MACRO(mod, ND_PIL); + ADD_INT_MACRO(mod, ND_GETBUF_FAIL); + ADD_INT_MACRO(mod, ND_GETBUF_UNDEFINED); + ADD_INT_MACRO(mod, ND_REDIRECT); + + ADD_INT_MACRO(mod, PyBUF_SIMPLE); + ADD_INT_MACRO(mod, PyBUF_WRITABLE); + ADD_INT_MACRO(mod, PyBUF_FORMAT); + ADD_INT_MACRO(mod, PyBUF_ND); + ADD_INT_MACRO(mod, PyBUF_STRIDES); + ADD_INT_MACRO(mod, PyBUF_INDIRECT); + ADD_INT_MACRO(mod, PyBUF_C_CONTIGUOUS); + ADD_INT_MACRO(mod, PyBUF_F_CONTIGUOUS); + ADD_INT_MACRO(mod, PyBUF_ANY_CONTIGUOUS); + ADD_INT_MACRO(mod, PyBUF_FULL); + ADD_INT_MACRO(mod, PyBUF_FULL_RO); + ADD_INT_MACRO(mod, PyBUF_RECORDS); + ADD_INT_MACRO(mod, PyBUF_RECORDS_RO); + ADD_INT_MACRO(mod, PyBUF_STRIDED); + ADD_INT_MACRO(mod, PyBUF_STRIDED_RO); + ADD_INT_MACRO(mod, PyBUF_CONTIG); + ADD_INT_MACRO(mod, PyBUF_CONTIG_RO); + + ADD_INT_MACRO(mod, PyBUF_READ); + ADD_INT_MACRO(mod, PyBUF_WRITE); #undef ADD_INT_MACRO @@ -2894,15 +2894,13 @@ _testbuffer_exec(PyObject *m) PyMODINIT_FUNC PyInit__testbuffer(void) { - PyObject *m; - - m = PyModule_Create(&_testbuffermodule); - if (m == NULL) { + PyObject *mod = PyModule_Create(&_testbuffermodule); + if (mod == NULL) { return NULL; } - if (_testbuffer_exec(m) < 0) { - Py_DECREF(m); + if (_testbuffer_exec(mod) < 0) { + Py_DECREF(mod); return NULL; } - return m; + return mod; } From 3d95285ef90efaf3b53924d982bfda501a214591 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 8 Feb 2024 21:25:03 +0300 Subject: [PATCH 7/7] Delete Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst --- .../2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst deleted file mode 100644 index 3c11cf76597c18..00000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-28-22-32-07.gh-issue-114672.9iEHAg.rst +++ /dev/null @@ -1,2 +0,0 @@ -Fix reference leaks during ``_testbuffer`` module initialization -in error handling.