Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-139400: Make sure that parent parsers outlive their subparsers in pyexpat#139403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
e1c04703b0afda8f70991a63bd3422417ec20caacdf8c58a3f1e3e361b2937d8c5a4e8a6f55204470cea3759ac730598a96d3c42466b77e0455787fa669454a91ed6bc056cfe12b89890541d6fFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -771,6 +771,42 @@ def resolve_entity(context, base, system_id, public_id): | ||
| self.assertEqual(handler_call_args, [("bar", "baz")]) | ||
| class ParentParserLifetimeTest(unittest.TestCase): | ||
| """ | ||
| Subparsers make use of their parent XML_Parser inside of Expat. | ||
| As a result, parent parsers need to outlive subparsers. | ||
| See https://github.com/python/cpython/issues/139400. | ||
| """ | ||
| def test_parent_parser_outlives_its_subparsers__single(self): | ||
| parser = expat.ParserCreate() | ||
| subparser = parser.ExternalEntityParserCreate(None) | ||
| # Now try to cause garbage collection of the parent parser | ||
| # while it's still being referenced by a related subparser. | ||
| del parser | ||
| def test_parent_parser_outlives_its_subparsers__multiple(self): | ||
| parser = expat.ParserCreate() | ||
| subparser_one = parser.ExternalEntityParserCreate(None) | ||
| subparser_two = parser.ExternalEntityParserCreate(None) | ||
| # Now try to cause garbage collection of the parent parser | ||
| # while it's still being referenced by a related subparser. | ||
| del parser | ||
| def test_parent_parser_outlives_its_subparsers__chain(self): | ||
| parser = expat.ParserCreate() | ||
| subparser = parser.ExternalEntityParserCreate(None) | ||
| subsubparser = subparser.ExternalEntityParserCreate(None) | ||
| # Now try to cause garbage collection of the parent parsers | ||
| # while they are still being referenced by a related subparser. | ||
| del parser | ||
hartwork marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| del subparser | ||
| class ReparseDeferralTest(unittest.TestCase): | ||
| def test_getter_setter_round_trip(self): | ||
| parser = expat.ParserCreate() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| :mod:`xml.parsers.expat`: Make sure that parent Expat parsers are only | ||
| garbage-collected once they are no longer referenced by subparsers created | ||
| by :meth:`~xml.parsers.expat.xmlparser.ExternalEntityParserCreate`. | ||
| Patch by Sebastian Pipping. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -76,6 +76,15 @@ typedef struct { | ||
| PyObject_HEAD | ||
| XML_Parser itself; | ||
| /* | ||
| * Strong reference to a parent `xmlparseobject` if this parser | ||
| * is a child parser. Set to NULL if this parser is a root parser. | ||
| * This is needed to keep the parent parser alive as long as it has | ||
| * at least one child parser. | ||
| * | ||
| * See https://github.com/python/cpython/issues/139400 for details. | ||
| */ | ||
| PyObject *parent; | ||
| int ordered_attributes; /* Return attributes as a list. */ | ||
| int specified_attributes; /* Report only specified attributes. */ | ||
| int in_callback; /* Is a callback active? */ | ||
| @@ -1065,6 +1074,11 @@ 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; | ||
| @@ -1074,18 +1088,21 @@ 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; | ||
| 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) { | ||
| Py_DECREF(new_parser); | ||
| Py_DECREF(self); | ||
| return PyErr_NoMemory(); | ||
| } | ||
| } | ||
| if (!new_parser->itself) { | ||
| Py_DECREF(new_parser); | ||
| Py_DECREF(self); | ||
| return PyErr_NoMemory(); | ||
| } | ||
| @@ -1099,6 +1116,7 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self, | ||
| new_parser->handlers = PyMem_New(PyObject *, i); | ||
| if (!new_parser->handlers) { | ||
| Py_DECREF(new_parser); | ||
| Py_DECREF(self); | ||
| return PyErr_NoMemory(); | ||
| } | ||
| clear_handlers(new_parser, 1); | ||
| @@ -1479,6 +1497,7 @@ newxmlparseobject(pyexpat_state *state, const char *encoding, | ||
| /* namespace_separator is either NULL or contains one char + \0 */ | ||
| self->itself = XML_ParserCreate_MM(encoding, &ExpatMemoryHandler, | ||
| namespace_separator); | ||
| self->parent = NULL; | ||
| if (self->itself == NULL) { | ||
| PyErr_SetString(PyExc_RuntimeError, | ||
| "XML_ParserCreate failed"); | ||
| @@ -1515,6 +1534,7 @@ xmlparse_traverse(PyObject *op, visitproc visit, void *arg) | ||
| for (size_t i = 0; handler_info[i].name != NULL; i++) { | ||
| Py_VISIT(self->handlers[i]); | ||
| } | ||
| Py_VISIT(self->parent); | ||
| Py_VISIT(Py_TYPE(op)); | ||
| return 0; | ||
| } | ||
| @@ -1525,6 +1545,10 @@ xmlparse_clear(PyObject *op) | ||
| xmlparseobject *self = xmlparseobject_CAST(op); | ||
| clear_handlers(self, 0); | ||
| Py_CLEAR(self->intern); | ||
| // NOTE: We cannot call Py_CLEAR(self->parent) prior to calling | ||
| // XML_ParserFree(self->itself), or a subparser could lose its parent | ||
| // XML_Parser while still making use of it internally. | ||
| // https://github.com/python/cpython/issues/139400 | ||
hartwork marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return 0; | ||
| } | ||
| @@ -1538,6 +1562,7 @@ xmlparse_dealloc(PyObject *op) | ||
| XML_ParserFree(self->itself); | ||
| } | ||
| self->itself = NULL; | ||
| Py_CLEAR(self->parent); | ||
picnixz marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (self->handlers != NULL) { | ||
| PyMem_Free(self->handlers); | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.