From ef85b24a78a4ac7a2e679fe019481cd9c1affd5b Mon Sep 17 00:00:00 2001 From: Vadim Levin Date: Thu, 27 Jan 2022 12:04:56 +0300 Subject: [PATCH] fix: wrong reference counter after module initialization --- modules/python/src2/cv2.cpp | 14 +++++++++----- modules/python/test/test_misc.py | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/modules/python/src2/cv2.cpp b/modules/python/src2/cv2.cpp index d4f1a5aa3a..a82086f315 100644 --- a/modules/python/src2/cv2.cpp +++ b/modules/python/src2/cv2.cpp @@ -2184,18 +2184,22 @@ static PyObject* createSubmodule(PyObject* parent_module, const std::string& nam submodule_name.c_str()); if (!submodule) { + /// Populates global modules dictionary and returns borrowed reference to it submodule = PyImport_AddModule(full_submodule_name.c_str()); + if (!submodule) + { + /// Return `PyImport_AddModule` NULL with an exception set on failure. + return NULL; + } + /// Populates parent module dictionary. Submodule lifetime should be managed + /// by the global modules dictionary and parent module dictionary, so Py_DECREF after + /// successfull call to the `PyDict_SetItemString` is redundant. if (PyDict_SetItemString(parent_module_dict, submodule_name.c_str(), submodule) < 0) { - Py_CLEAR(submodule); return PyErr_Format(PyExc_ImportError, "Can't register a submodule '%s' (full name: '%s')", submodule_name.c_str(), full_submodule_name.c_str() ); } - /// PyDict_SetItemString doesn't steal a reference so the reference counter - /// of the submodule should be decremented to bind submodule lifetime to the - /// parent module - Py_DECREF(submodule); } submodule_name_start = submodule_name_end + 1; diff --git a/modules/python/test/test_misc.py b/modules/python/test/test_misc.py index f9e75e8fe4..41e5c6ba4b 100644 --- a/modules/python/test/test_misc.py +++ b/modules/python/test/test_misc.py @@ -602,10 +602,18 @@ class Arguments(NewOpenCVTests): msg="Module is not generated for nested namespace") self.assertTrue(hasattr(cv.utils.nested, "testEchoBooleanFunction"), msg="Function in nested module is not available") - self.assertEqual(sys.getrefcount(cv.utils.nested), 2, - msg="Nested submodule lifetime should be managed by " - "the parent module so the reference count should be " - "2, because `getrefcount` temporary increases it.") + + # Nested submodule is managed by the global submodules dictionary + # and parent native module + expected_ref_count = 2 + + # `getrefcount` temporary increases reference counter by 1 + actual_ref_count = sys.getrefcount(cv.utils.nested) - 1 + + self.assertEqual(actual_ref_count, expected_ref_count, + msg="Nested submodule reference counter has wrong value\n" + "Expected: {}. Actual: {}".format(expected_ref_count, actual_ref_count)) + for flag in (True, False): self.assertEqual(flag, cv.utils.nested.testEchoBooleanFunction(flag), msg="Function in nested module returns wrong result") -- GitLab