gh-144984: Address review: use Py_NewRef(self) for parent, add setUpClass

Author: raminfp

Lib/test/test_pyexpat.py

@@ -831,23 +831,28 @@ class ExternalEntityParserCreateErrorTest(unittest.TestCase):
     See https://github.com/python/cpython/issues/144984.
     """
 
+    @classmethod
+    def setUpClass(cls):
+        cls._testcapi = import_helper.import_module('_testcapi')
+
     def test_error_path_no_crash(self):
         # When an allocation inside ExternalEntityParserCreate fails,
         # the partially-initialized subparser is deallocated.  This
         # must not dereference NULL handlers or double-decrement the
         # parent parser's refcount.
-        _testcapi = import_helper.import_module('_testcapi')
         parser = expat.ParserCreate()
         parser.buffer_text = True
         rc_before = sys.getrefcount(parser)
 
-        _testcapi.set_nomemory(1, 10)
+        self._testcapi.set_nomemory(1, 10)
+        raised = False
         try:
             parser.ExternalEntityParserCreate(None)
         except MemoryError:
-            pass
+            raised = True
         finally:
-            _testcapi.remove_mem_hooks()
+            self._testcapi.remove_mem_hooks()
+        self.assertTrue(raised, "MemoryError not raised")
 
         rc_after = sys.getrefcount(parser)
         self.assertEqual(rc_after, rc_before)

Modules/pyexpat.c

@@ -1076,11 +1076,6 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
         return NULL;
     }
 
-    // The new subparser will make use of the parent XML_Parser inside of Expat.
-    // So we need to take subparsers into account with the reference counting
-    // of their parent parser.
-    Py_INCREF(self);
-
     new_parser->buffer_size = self->buffer_size;
     new_parser->buffer_used = 0;
     new_parser->buffer = NULL;
@@ -1090,23 +1085,22 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
     new_parser->ns_prefixes = self->ns_prefixes;
     new_parser->itself = XML_ExternalEntityParserCreate(self->itself, context,
                                                         encoding);
-    new_parser->parent = (PyObject *)self;
+    // The new subparser will make use of the parent XML_Parser inside of Expat.
+    // So we need to take subparsers into account with the reference counting
+    // of their parent parser.
+    new_parser->parent = Py_NewRef(self);
     new_parser->handlers = 0;
     new_parser->intern = Py_XNewRef(self->intern);
 
     if (self->buffer != NULL) {
         new_parser->buffer = PyMem_Malloc(new_parser->buffer_size);
         if (new_parser->buffer == NULL) {
-            new_parser->parent = NULL;
             Py_DECREF(new_parser);
-            Py_DECREF(self);
             return PyErr_NoMemory();
         }
     }
     if (!new_parser->itself) {
-        new_parser->parent = NULL;
         Py_DECREF(new_parser);
-        Py_DECREF(self);
         return PyErr_NoMemory();
     }
 
@@ -1119,9 +1113,7 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
 
     new_parser->handlers = PyMem_New(PyObject *, i);
     if (!new_parser->handlers) {
-        new_parser->parent = NULL;
         Py_DECREF(new_parser);
-        Py_DECREF(self);
         return PyErr_NoMemory();
     }
     clear_handlers(new_parser, 1);