From 644e883c58b3041569acb068c7ad68a61b563652 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 11:10:41 +0900 Subject: [PATCH 01/14] gh-84436 Add intergration C API tests for immortal objects --- Modules/Setup.stdlib.in | 2 +- Modules/_testcapi/parts.h | 1 + Modules/_testcapimodule.c | 3 +++ PCbuild/_testcapi.vcxproj | 3 ++- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Modules/Setup.stdlib.in b/Modules/Setup.stdlib.in index fe1b9f8f5380c1..a90c1e96ef0231 100644 --- a/Modules/Setup.stdlib.in +++ b/Modules/Setup.stdlib.in @@ -169,7 +169,7 @@ @MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c @MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c @MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c -@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/getargs.c _testcapi/pytime.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/pyos.c +@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/getargs.c _testcapi/pytime.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/pyos.c _testcapi/immortal.c @MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c # Some testing modules MUST be built as shared libraries. diff --git a/Modules/_testcapi/parts.h b/Modules/_testcapi/parts.h index 60ec81dad2ba9e..4d2d6832a827ae 100644 --- a/Modules/_testcapi/parts.h +++ b/Modules/_testcapi/parts.h @@ -39,6 +39,7 @@ int _PyTestCapi_Init_Structmember(PyObject *module); int _PyTestCapi_Init_Exceptions(PyObject *module); int _PyTestCapi_Init_Code(PyObject *module); int _PyTestCapi_Init_PyOS(PyObject *module); +int _PyTestCapi_Init_Immortal(PyObject *module); #ifdef LIMITED_API_AVAILABLE int _PyTestCapi_Init_VectorcallLimited(PyObject *module); diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index c1892f6fa0a4b8..b4750eb936d798 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4210,6 +4210,9 @@ PyInit__testcapi(void) if (_PyTestCapi_Init_PyOS(m) < 0) { return NULL; } + if (_PyTestCapi_Init_Immortal(m) < 0) { + return NULL; + } #ifndef LIMITED_API_AVAILABLE PyModule_AddObjectRef(m, "LIMITED_API_AVAILABLE", Py_False); diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index 439cd687fda61d..3484637decf95c 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -110,6 +110,7 @@ + @@ -127,4 +128,4 @@ - \ No newline at end of file + From 2eea452fe624a56ef37400bb1eb2d3febbd3698c Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 11:13:57 +0900 Subject: [PATCH 02/14] Update PCbuild/_testcapi.vcxproj --- PCbuild/_testcapi.vcxproj | 1 + 1 file changed, 1 insertion(+) diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index 3484637decf95c..7d7b6aecdb88ac 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -129,3 +129,4 @@ + From 1c702112dcf7bd2d07290816f609ca640fd79413 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 11:16:14 +0900 Subject: [PATCH 03/14] Windows format --- PCbuild/_testcapi.vcxproj | 1 - 1 file changed, 1 deletion(-) diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index 7d7b6aecdb88ac..3484637decf95c 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -129,4 +129,3 @@ - From 6a83baeccc5a53b2711a71d2b83959fa4cdea2de Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 11:30:01 +0900 Subject: [PATCH 04/14] Add immortal.c --- Modules/_testcapi/immortal.c | 105 +++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 Modules/_testcapi/immortal.c diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c new file mode 100644 index 00000000000000..1416f164bc4e98 --- /dev/null +++ b/Modules/_testcapi/immortal.c @@ -0,0 +1,105 @@ +#include "parts.h" + +static PyObject* +test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject* objects[] = {Py_True, Py_False}; + Py_ssize_t n = sizeof(objects) / sizeof(objects[0]); + for(Py_ssize_t i = 0; i < n; i++) { + PyObject* obj = objects[i]; + if (!_Py_IsImmortal(obj)) { + PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", obj); + return NULL; + } + Py_ssize_t old_count = Py_REFCNT(obj); + for (int i = 0; i < 10000; i++) { + Py_DECREF(obj); + } + Py_ssize_t current_count = Py_REFCNT(obj); + if (old_count != current_count) { + PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); + return NULL; + } + } + Py_RETURN_NONE; +} + +static PyObject * +test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject* none = Py_None; + if (!_Py_IsImmortal(none)) { + PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", none); + return NULL; + } + Py_ssize_t old_count = Py_REFCNT(none); + for (int i = 0; i < 10000; i++) { + Py_DECREF(none); + } + Py_ssize_t current_count = Py_REFCNT(none); + if (old_count != current_count) { + PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); + return NULL; + } + Py_RETURN_NONE; +} + +static PyObject * +test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + for(int i = -5; i < 255; i++) { + PyObject *small_int = PyLong_FromLong(i); + if (!_Py_IsImmortal(small_int)) { + PyErr_Format(PyExc_RuntimeError, "Small int(%d) object should be the immportal object.", i); + return NULL; + } + Py_ssize_t old_count = Py_REFCNT(small_int); + for (int i = 0; i < 10000; i++) { + Py_DECREF(small_int); + } + Py_ssize_t current_count = Py_REFCNT(small_int); + if (old_count != current_count) { + PyErr_Format(PyExc_RuntimeError, "Reference count of %d should not be changed.", i); + return NULL; + } + } + Py_RETURN_NONE; +} + +static PyObject * +test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject* ellipsis = Py_Ellipsis; + if (!_Py_IsImmortal(ellipsis)) { + PyErr_SetString(PyExc_RuntimeError, "Ellipsis object should be the immportal object."); + return NULL; + } + Py_ssize_t old_count = Py_REFCNT(ellipsis); + for (int i = 0; i < 10000; i++) { + Py_DECREF(ellipsis); + } + Py_ssize_t current_count = Py_REFCNT(ellipsis); + if (old_count != current_count) { + PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); + return NULL; + } + Py_RETURN_NONE; +} + + +static PyMethodDef test_methods[] = { + {"test_immortal_bool", test_immortal_bool, METH_NOARGS}, + {"test_immortal_none", test_immortal_none, METH_NOARGS}, + {"test_immortal_small_ints", test_immortal_small_ints, METH_NOARGS}, + {"test_immortal_ellipsis", test_immortal_ellipsis, METH_NOARGS}, + {NULL}, +}; + +int +_PyTestCapi_Init_Immortal(PyObject *mod) +{ + if (PyModule_AddFunctions(mod, test_methods) < 0) { + return -1; + } + return 0; +} \ No newline at end of file From 94ca054672e057db9b0e49db462ae0b7b8c57974 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 11:31:19 +0900 Subject: [PATCH 05/14] Add test_immortal.py --- Lib/test/test_capi/test_immortal.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 Lib/test/test_capi/test_immortal.py diff --git a/Lib/test/test_capi/test_immortal.py b/Lib/test/test_capi/test_immortal.py new file mode 100644 index 00000000000000..fa7c0bffb8bd84 --- /dev/null +++ b/Lib/test/test_capi/test_immortal.py @@ -0,0 +1,19 @@ +import unittest +from test.support import import_helper + +_testcapi = import_helper.import_module('_testcapi') + + +class TestCAPI(unittest.TestCase): + def test_immortal_bool(self): + _testcapi.test_immortal_bool() + + def test_immortal_none(self): + _testcapi.test_immortal_none() + + def test_immortal_ellipsis(self): + _testcapi.test_immortal_ellipsis() + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file From 73daa12b0b308312c319d7b1f44b2ffc1d22ae2d Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 11:43:18 +0900 Subject: [PATCH 06/14] fix format --- PCbuild/_testcapi.vcxproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index 3484637decf95c..21941247eb9692 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -128,4 +128,4 @@ - + \ No newline at end of file From e5b8e45b2b5d52a57e95d766b0eba8a4770f9480 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 12:21:27 +0900 Subject: [PATCH 07/14] make patchcheck --- Lib/test/test_capi/test_immortal.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi/test_immortal.py b/Lib/test/test_capi/test_immortal.py index fa7c0bffb8bd84..9279fc5fd0f42b 100644 --- a/Lib/test/test_capi/test_immortal.py +++ b/Lib/test/test_capi/test_immortal.py @@ -10,10 +10,10 @@ def test_immortal_bool(self): def test_immortal_none(self): _testcapi.test_immortal_none() - + def test_immortal_ellipsis(self): _testcapi.test_immortal_ellipsis() if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() From 2f4ee5f9dc102e60341cee41df9ca02826eb4760 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 12:23:11 +0900 Subject: [PATCH 08/14] fix format --- Modules/_testcapi/immortal.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index 1416f164bc4e98..51ec58fd90e9bf 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -9,7 +9,7 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) PyObject* obj = objects[i]; if (!_Py_IsImmortal(obj)) { PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", obj); - return NULL; + return NULL; } Py_ssize_t old_count = Py_REFCNT(obj); for (int i = 0; i < 10000; i++) { @@ -18,7 +18,7 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_ssize_t current_count = Py_REFCNT(obj); if (old_count != current_count) { PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); - return NULL; + return NULL; } } Py_RETURN_NONE; @@ -30,7 +30,7 @@ test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) PyObject* none = Py_None; if (!_Py_IsImmortal(none)) { PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", none); - return NULL; + return NULL; } Py_ssize_t old_count = Py_REFCNT(none); for (int i = 0; i < 10000; i++) { @@ -39,7 +39,7 @@ test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_ssize_t current_count = Py_REFCNT(none); if (old_count != current_count) { PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); - return NULL; + return NULL; } Py_RETURN_NONE; } @@ -51,7 +51,7 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) PyObject *small_int = PyLong_FromLong(i); if (!_Py_IsImmortal(small_int)) { PyErr_Format(PyExc_RuntimeError, "Small int(%d) object should be the immportal object.", i); - return NULL; + return NULL; } Py_ssize_t old_count = Py_REFCNT(small_int); for (int i = 0; i < 10000; i++) { @@ -60,7 +60,7 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_ssize_t current_count = Py_REFCNT(small_int); if (old_count != current_count) { PyErr_Format(PyExc_RuntimeError, "Reference count of %d should not be changed.", i); - return NULL; + return NULL; } } Py_RETURN_NONE; @@ -72,7 +72,7 @@ test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) PyObject* ellipsis = Py_Ellipsis; if (!_Py_IsImmortal(ellipsis)) { PyErr_SetString(PyExc_RuntimeError, "Ellipsis object should be the immportal object."); - return NULL; + return NULL; } Py_ssize_t old_count = Py_REFCNT(ellipsis); for (int i = 0; i < 10000; i++) { @@ -81,7 +81,7 @@ test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_ssize_t current_count = Py_REFCNT(ellipsis); if (old_count != current_count) { PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); - return NULL; + return NULL; } Py_RETURN_NONE; } @@ -97,9 +97,9 @@ static PyMethodDef test_methods[] = { int _PyTestCapi_Init_Immortal(PyObject *mod) -{ +{ if (PyModule_AddFunctions(mod, test_methods) < 0) { return -1; } return 0; -} \ No newline at end of file +} From 55a394b36869e73662f33e04cfae47805e8ef0cc Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 14:47:46 +0900 Subject: [PATCH 09/14] address code review --- Modules/_testcapi/immortal.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index 51ec58fd90e9bf..0d8e93badc61e0 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -4,7 +4,7 @@ static PyObject* test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject* objects[] = {Py_True, Py_False}; - Py_ssize_t n = sizeof(objects) / sizeof(objects[0]); + Py_ssize_t n = Py_ARRAY_LENGTH(objects); for(Py_ssize_t i = 0; i < n; i++) { PyObject* obj = objects[i]; if (!_Py_IsImmortal(obj)) { @@ -12,7 +12,7 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) return NULL; } Py_ssize_t old_count = Py_REFCNT(obj); - for (int i = 0; i < 10000; i++) { + for (int j = 0; j < 10000; j++) { Py_DECREF(obj); } Py_ssize_t current_count = Py_REFCNT(obj); @@ -27,16 +27,15 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject* none = Py_None; - if (!_Py_IsImmortal(none)) { - PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", none); + if (!_Py_IsImmortal(Py_None)) { + PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", Py_None); return NULL; } - Py_ssize_t old_count = Py_REFCNT(none); + Py_ssize_t old_count = Py_REFCNT(Py_None); for (int i = 0; i < 10000; i++) { - Py_DECREF(none); + Py_DECREF(Py_None); } - Py_ssize_t current_count = Py_REFCNT(none); + Py_ssize_t current_count = Py_REFCNT(Py_None); if (old_count != current_count) { PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); return NULL; @@ -54,7 +53,7 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) return NULL; } Py_ssize_t old_count = Py_REFCNT(small_int); - for (int i = 0; i < 10000; i++) { + for (int j = 0; j < 10000; j++) { Py_DECREF(small_int); } Py_ssize_t current_count = Py_REFCNT(small_int); @@ -69,16 +68,15 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject* ellipsis = Py_Ellipsis; - if (!_Py_IsImmortal(ellipsis)) { + if (!_Py_IsImmortal(Py_Ellipsis)) { PyErr_SetString(PyExc_RuntimeError, "Ellipsis object should be the immportal object."); return NULL; } - Py_ssize_t old_count = Py_REFCNT(ellipsis); + Py_ssize_t old_count = Py_REFCNT(Py_Ellipsis); for (int i = 0; i < 10000; i++) { - Py_DECREF(ellipsis); + Py_DECREF(Py_Ellipsis); } - Py_ssize_t current_count = Py_REFCNT(ellipsis); + Py_ssize_t current_count = Py_REFCNT(Py_Ellipsis); if (old_count != current_count) { PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); return NULL; From 4fb67192280b979dcc7aeec892066b038b0c5af8 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 14:54:09 +0900 Subject: [PATCH 10/14] Adjust small int range --- Modules/_testcapi/immortal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index 0d8e93badc61e0..db3a1af58de150 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -46,7 +46,7 @@ test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) { - for(int i = -5; i < 255; i++) { + for(int i = -5; i <= 256; i++) { PyObject *small_int = PyLong_FromLong(i); if (!_Py_IsImmortal(small_int)) { PyErr_Format(PyExc_RuntimeError, "Small int(%d) object should be the immportal object.", i); From e737489e8b49fa82f1850d2d0776ad73897fdf09 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 20:19:22 +0900 Subject: [PATCH 11/14] Address code review --- Modules/_testcapi/immortal.c | 46 +++++++++--------------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index db3a1af58de150..d79781250dac94 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -1,25 +1,19 @@ #include "parts.h" -static PyObject* +static PyObject * test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject* objects[] = {Py_True, Py_False}; Py_ssize_t n = Py_ARRAY_LENGTH(objects); - for(Py_ssize_t i = 0; i < n; i++) { + for (Py_ssize_t i = 0; i < n; i++) { PyObject* obj = objects[i]; - if (!_Py_IsImmortal(obj)) { - PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", obj); - return NULL; - } + assert(_Py_IsImmortal(obj)); Py_ssize_t old_count = Py_REFCNT(obj); for (int j = 0; j < 10000; j++) { Py_DECREF(obj); } Py_ssize_t current_count = Py_REFCNT(obj); - if (old_count != current_count) { - PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); - return NULL; - } + assert(old_count == current_count); } Py_RETURN_NONE; } @@ -27,40 +21,28 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) { - if (!_Py_IsImmortal(Py_None)) { - PyErr_Format(PyExc_RuntimeError, "%R should be the immportal object.", Py_None); - return NULL; - } + assert(_Py_IsImmortal(Py_None)); Py_ssize_t old_count = Py_REFCNT(Py_None); for (int i = 0; i < 10000; i++) { Py_DECREF(Py_None); } Py_ssize_t current_count = Py_REFCNT(Py_None); - if (old_count != current_count) { - PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); - return NULL; - } + assert(old_count == current_count); Py_RETURN_NONE; } static PyObject * test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) { - for(int i = -5; i <= 256; i++) { + for (int i = -5; i <= 256; i++) { PyObject *small_int = PyLong_FromLong(i); - if (!_Py_IsImmortal(small_int)) { - PyErr_Format(PyExc_RuntimeError, "Small int(%d) object should be the immportal object.", i); - return NULL; - } + assert(_Py_IsImmortal(small_int)); Py_ssize_t old_count = Py_REFCNT(small_int); for (int j = 0; j < 10000; j++) { Py_DECREF(small_int); } Py_ssize_t current_count = Py_REFCNT(small_int); - if (old_count != current_count) { - PyErr_Format(PyExc_RuntimeError, "Reference count of %d should not be changed.", i); - return NULL; - } + assert(old_count == current_count); } Py_RETURN_NONE; } @@ -68,19 +50,13 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) { - if (!_Py_IsImmortal(Py_Ellipsis)) { - PyErr_SetString(PyExc_RuntimeError, "Ellipsis object should be the immportal object."); - return NULL; - } + assert(_Py_IsImmortal(Py_Ellipsis)); Py_ssize_t old_count = Py_REFCNT(Py_Ellipsis); for (int i = 0; i < 10000; i++) { Py_DECREF(Py_Ellipsis); } Py_ssize_t current_count = Py_REFCNT(Py_Ellipsis); - if (old_count != current_count) { - PyErr_SetString(PyExc_RuntimeError, "Reference count should not be changed."); - return NULL; - } + assert(old_count == current_count); Py_RETURN_NONE; } From b03def1d353c4f3fec88f68e9e59674b8fe52ddc Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 28 Apr 2023 20:20:58 +0900 Subject: [PATCH 12/14] nit --- Modules/_testcapi/immortal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index d79781250dac94..8a6068481cf979 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -3,7 +3,7 @@ static PyObject * test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject* objects[] = {Py_True, Py_False}; + PyObject *objects[] = {Py_True, Py_False}; Py_ssize_t n = Py_ARRAY_LENGTH(objects); for (Py_ssize_t i = 0; i < n; i++) { PyObject* obj = objects[i]; From e0295b5fcefe8f787bb6cd0e36a58a83e72b8ae7 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 2 May 2023 19:13:36 +0900 Subject: [PATCH 13/14] Address code review --- Modules/_testcapi/immortal.c | 45 ++++++++++++------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index 8a6068481cf979..6cc4bc6abdb113 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -1,19 +1,23 @@ #include "parts.h" +int verify_immortality(PyObject *object) +{ + assert(_Py_IsImmortal(object)); + Py_ssize_t old_count = Py_REFCNT(object); + for (int j = 0; j < 10000; j++) { + Py_DECREF(object); + } + Py_ssize_t current_count = Py_REFCNT(object); + return old_count == current_count; +} + static PyObject * test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *objects[] = {Py_True, Py_False}; Py_ssize_t n = Py_ARRAY_LENGTH(objects); for (Py_ssize_t i = 0; i < n; i++) { - PyObject* obj = objects[i]; - assert(_Py_IsImmortal(obj)); - Py_ssize_t old_count = Py_REFCNT(obj); - for (int j = 0; j < 10000; j++) { - Py_DECREF(obj); - } - Py_ssize_t current_count = Py_REFCNT(obj); - assert(old_count == current_count); + assert(verify_immortality(objects[i])); } Py_RETURN_NONE; } @@ -21,13 +25,7 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) { - assert(_Py_IsImmortal(Py_None)); - Py_ssize_t old_count = Py_REFCNT(Py_None); - for (int i = 0; i < 10000; i++) { - Py_DECREF(Py_None); - } - Py_ssize_t current_count = Py_REFCNT(Py_None); - assert(old_count == current_count); + assert(verify_immortality(Py_None)); Py_RETURN_NONE; } @@ -35,14 +33,7 @@ static PyObject * test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) { for (int i = -5; i <= 256; i++) { - PyObject *small_int = PyLong_FromLong(i); - assert(_Py_IsImmortal(small_int)); - Py_ssize_t old_count = Py_REFCNT(small_int); - for (int j = 0; j < 10000; j++) { - Py_DECREF(small_int); - } - Py_ssize_t current_count = Py_REFCNT(small_int); - assert(old_count == current_count); + assert(verify_immortality(PyLong_FromLong(i))); } Py_RETURN_NONE; } @@ -50,13 +41,7 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) static PyObject * test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) { - assert(_Py_IsImmortal(Py_Ellipsis)); - Py_ssize_t old_count = Py_REFCNT(Py_Ellipsis); - for (int i = 0; i < 10000; i++) { - Py_DECREF(Py_Ellipsis); - } - Py_ssize_t current_count = Py_REFCNT(Py_Ellipsis); - assert(old_count == current_count); + assert(verify_immortality(Py_Ellipsis)); Py_RETURN_NONE; } From 5538ed4643db9eadb22f55b6d43834be0de11e49 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 2 May 2023 19:17:56 +0900 Subject: [PATCH 14/14] Refactor test codes and fix test_immortal.py --- Lib/test/test_capi/test_immortal.py | 11 ++++------- Modules/_testcapi/immortal.c | 23 +++-------------------- 2 files changed, 7 insertions(+), 27 deletions(-) diff --git a/Lib/test/test_capi/test_immortal.py b/Lib/test/test_capi/test_immortal.py index 9279fc5fd0f42b..ef5d32b7f01935 100644 --- a/Lib/test/test_capi/test_immortal.py +++ b/Lib/test/test_capi/test_immortal.py @@ -5,14 +5,11 @@ class TestCAPI(unittest.TestCase): - def test_immortal_bool(self): - _testcapi.test_immortal_bool() + def test_immortal_builtins(self): + _testcapi.test_immortal_builtins() - def test_immortal_none(self): - _testcapi.test_immortal_none() - - def test_immortal_ellipsis(self): - _testcapi.test_immortal_ellipsis() + def test_immortal_small_ints(self): + _testcapi.test_immortal_small_ints() if __name__ == "__main__": diff --git a/Modules/_testcapi/immortal.c b/Modules/_testcapi/immortal.c index 6cc4bc6abdb113..10e1733d08a9ea 100644 --- a/Modules/_testcapi/immortal.c +++ b/Modules/_testcapi/immortal.c @@ -12,9 +12,9 @@ int verify_immortality(PyObject *object) } static PyObject * -test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) +test_immortal_builtins(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyObject *objects[] = {Py_True, Py_False}; + PyObject *objects[] = {Py_True, Py_False, Py_None, Py_Ellipsis}; Py_ssize_t n = Py_ARRAY_LENGTH(objects); for (Py_ssize_t i = 0; i < n; i++) { assert(verify_immortality(objects[i])); @@ -22,13 +22,6 @@ test_immortal_bool(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_RETURN_NONE; } -static PyObject * -test_immortal_none(PyObject *self, PyObject *Py_UNUSED(ignored)) -{ - assert(verify_immortality(Py_None)); - Py_RETURN_NONE; -} - static PyObject * test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) { @@ -38,19 +31,9 @@ test_immortal_small_ints(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_RETURN_NONE; } -static PyObject * -test_immortal_ellipsis(PyObject *self, PyObject *Py_UNUSED(ignored)) -{ - assert(verify_immortality(Py_Ellipsis)); - Py_RETURN_NONE; -} - - static PyMethodDef test_methods[] = { - {"test_immortal_bool", test_immortal_bool, METH_NOARGS}, - {"test_immortal_none", test_immortal_none, METH_NOARGS}, + {"test_immortal_builtins", test_immortal_builtins, METH_NOARGS}, {"test_immortal_small_ints", test_immortal_small_ints, METH_NOARGS}, - {"test_immortal_ellipsis", test_immortal_ellipsis, METH_NOARGS}, {NULL}, };