[Python-checkins] [3.6] bpo-31499, xml.etree: Fix xmlparser_gc_clear() crash (GH-3641) (#3645)

[Victor Stinner]

https://github.com/python/cpython/commit/8afd7ab12d7f8915b549cf04af384b495ec73d22
commit: 8afd7ab12d7f8915b549cf04af384b495ec73d22
branch: 3.6
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Victor Stinner <victor.stinner at gmail.com>
date: 2017-09-18T05:48:23-07:00
summary:

[3.6] bpo-31499, xml.etree: Fix xmlparser_gc_clear() crash (GH-3641) (#3645)

* bpo-31499, xml.etree: Fix xmlparser_gc_clear() crash

xml.etree: xmlparser_gc_clear() now sets self.parser to NULL to prevent a
crash in xmlparser_dealloc() if xmlparser_gc_clear() was called previously
by the garbage collector, because the parser was part of a reference cycle.

Co-Authored-By: Serhiy Storchaka <storchaka at gmail.com>
(cherry picked from commit e727d41ffcd91b21ce82026ec8c8381d34a16209)

files:
A Misc/NEWS.d/next/Library/2017-09-18-10-57-04.bpo-31499.BydYhf.rst
M Lib/test/test_xml_etree_c.py
M Modules/_elementtree.c

diff --git a/Lib/test/test_xml_etree_c.py b/Lib/test/test_xml_etree_c.py
index 7c60699f912..b2200d31f59 100644
--- a/Lib/test/test_xml_etree_c.py
+++ b/Lib/test/test_xml_etree_c.py
@@ -64,6 +64,26 @@ def test_trashcan(self):
         del root
         support.gc_collect()
 
+    def test_parser_ref_cycle(self):
+        # bpo-31499: xmlparser_dealloc() crashed with a segmentation fault when
+        # xmlparser_gc_clear() was called previously by the garbage collector,
+        # when the parser was part of a reference cycle.
+
+        def parser_ref_cycle():
+            parser = cET.XMLParser()
+            # Create a reference cycle using an exception to keep the frame
+            # alive, so the parser will be destroyed by the garbage collector
+            try:
+                raise ValueError
+            except ValueError as exc:
+                err = exc
+
+        # Create a parser part of reference cycle
+        parser_ref_cycle()
+        # Trigger an explicit garbage collection to break the reference cycle
+        # and so destroy the parser
+        support.gc_collect()
+
 
 @unittest.skipUnless(cET, 'requires _elementtree')
 class TestAliasWorking(unittest.TestCase):
diff --git a/Misc/NEWS.d/next/Library/2017-09-18-10-57-04.bpo-31499.BydYhf.rst b/Misc/NEWS.d/next/Library/2017-09-18-10-57-04.bpo-31499.BydYhf.rst
new file mode 100644
index 00000000000..22af29fdecb
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-09-18-10-57-04.bpo-31499.BydYhf.rst
@@ -0,0 +1 @@
+xml.etree: Fix a crash when a parser is part of a reference cycle.
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index cf3e687c8c6..a6d6c61dd24 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -3367,7 +3367,11 @@ xmlparser_gc_traverse(XMLParserObject *self, visitproc visit, void *arg)
 static int
 xmlparser_gc_clear(XMLParserObject *self)
 {
-    EXPAT(ParserFree)(self->parser);
+    if (self->parser != NULL) {
+        XML_Parser parser = self->parser;
+        self->parser = NULL;
+        EXPAT(ParserFree)(parser);
+    }
 
     Py_CLEAR(self->handle_close);
     Py_CLEAR(self->handle_pi);