Merge pull request #21525 from VadimLevin:dev/vlevin/pysubmodules-refcounter-fix

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;

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")