changeset: 99654:e09cb2af3092 parent: 99652:e1418fc70e82 parent: 99653:00b6a13cfd70 user: Serhiy Storchaka date: Mon Dec 21 11:11:12 2015 +0200 files: Lib/test/test_xml_etree.py Lib/xml/etree/ElementTree.py Misc/NEWS Modules/_elementtree.c description: Issue #25902: Fixed various refcount issues in ElementTree iteration. diff -r e1418fc70e82 -r e09cb2af3092 Lib/test/test_xml_etree.py --- a/Lib/test/test_xml_etree.py Sun Dec 20 22:47:04 2015 -0800 +++ b/Lib/test/test_xml_etree.py Mon Dec 21 11:11:12 2015 +0200 @@ -1668,6 +1668,57 @@ ET.register_namespace('test10777', 'http://myuri/') ET.register_namespace('test10777', 'http://myuri/') + def test_lost_text(self): + # Issue #25902: Borrowed text can disappear + class Text: + def __bool__(self): + e.text = 'changed' + return True + + e = ET.Element('tag') + e.text = Text() + i = e.itertext() + t = next(i) + self.assertIsInstance(t, Text) + self.assertIsInstance(e.text, str) + self.assertEqual(e.text, 'changed') + + def test_lost_tail(self): + # Issue #25902: Borrowed tail can disappear + class Text: + def __bool__(self): + e[0].tail = 'changed' + return True + + e = ET.Element('root') + e.append(ET.Element('tag')) + e[0].tail = Text() + i = e.itertext() + t = next(i) + self.assertIsInstance(t, Text) + self.assertIsInstance(e[0].tail, str) + self.assertEqual(e[0].tail, 'changed') + + def test_lost_elem(self): + # Issue #25902: Borrowed element can disappear + class Tag: + def __eq__(self, other): + e[0] = ET.Element('changed') + next(i) + return True + + e = ET.Element('root') + e.append(ET.Element(Tag())) + e.append(ET.Element('tag')) + i = e.iter('tag') + try: + t = next(i) + except ValueError: + self.skipTest('generators are not reentrant') + self.assertIsInstance(t.tag, Tag) + self.assertIsInstance(e[0].tag, str) + self.assertEqual(e[0].tag, 'changed') + # -------------------------------------------------------------------- diff -r e1418fc70e82 -r e09cb2af3092 Lib/xml/etree/ElementTree.py --- a/Lib/xml/etree/ElementTree.py Sun Dec 20 22:47:04 2015 -0800 +++ b/Lib/xml/etree/ElementTree.py Mon Dec 21 11:11:12 2015 +0200 @@ -429,12 +429,14 @@ tag = self.tag if not isinstance(tag, str) and tag is not None: return - if self.text: - yield self.text + t = self.text + if t: + yield t for e in self: yield from e.itertext() - if e.tail: - yield e.tail + t = e.tail + if t: + yield t def SubElement(parent, tag, attrib={}, **extra): diff -r e1418fc70e82 -r e09cb2af3092 Misc/NEWS --- a/Misc/NEWS Sun Dec 20 22:47:04 2015 -0800 +++ b/Misc/NEWS Mon Dec 21 11:11:12 2015 +0200 @@ -115,6 +115,8 @@ Library ------- +- Issue #25902: Fixed various refcount issues in ElementTree iteration. + - Issue #22227: The TarFile iterator is reimplemented using generator. This implementation is simpler that using class. diff -r e1418fc70e82 -r e09cb2af3092 Modules/_elementtree.c --- a/Modules/_elementtree.c Sun Dec 20 22:47:04 2015 -0800 +++ b/Modules/_elementtree.c Mon Dec 21 11:11:12 2015 +0200 @@ -2070,6 +2070,7 @@ ElementObject *cur_parent; Py_ssize_t child_index; int rc; + ElementObject *elem; while (1) { /* Handle the case reached in the beginning and end of iteration, where @@ -2083,38 +2084,47 @@ PyErr_SetNone(PyExc_StopIteration); return NULL; } else { + elem = it->root_element; it->parent_stack = parent_stack_push_new(it->parent_stack, - it->root_element); + elem); if (!it->parent_stack) { PyErr_NoMemory(); return NULL; } + Py_INCREF(elem); it->root_done = 1; rc = (it->sought_tag == Py_None); if (!rc) { - rc = PyObject_RichCompareBool(it->root_element->tag, + rc = PyObject_RichCompareBool(elem->tag, it->sought_tag, Py_EQ); - if (rc < 0) + if (rc < 0) { + Py_DECREF(elem); return NULL; + } } if (rc) { if (it->gettext) { - PyObject *text = element_get_text(it->root_element); - if (!text) + PyObject *text = element_get_text(elem); + if (!text) { + Py_DECREF(elem); return NULL; + } + Py_INCREF(text); + Py_DECREF(elem); rc = PyObject_IsTrue(text); + if (rc > 0) + return text; + Py_DECREF(text); if (rc < 0) return NULL; - if (rc) { - Py_INCREF(text); - return text; - } } else { - Py_INCREF(it->root_element); - return (PyObject *)it->root_element; + return (PyObject *)elem; } } + else { + Py_DECREF(elem); + } } } @@ -2124,54 +2134,68 @@ cur_parent = it->parent_stack->parent; child_index = it->parent_stack->child_index; if (cur_parent->extra && child_index < cur_parent->extra->length) { - ElementObject *child = (ElementObject *) - cur_parent->extra->children[child_index]; + elem = (ElementObject *)cur_parent->extra->children[child_index]; it->parent_stack->child_index++; it->parent_stack = parent_stack_push_new(it->parent_stack, - child); + elem); if (!it->parent_stack) { PyErr_NoMemory(); return NULL; } + Py_INCREF(elem); if (it->gettext) { - PyObject *text = element_get_text(child); - if (!text) + PyObject *text = element_get_text(elem); + if (!text) { + Py_DECREF(elem); return NULL; + } + Py_INCREF(text); + Py_DECREF(elem); rc = PyObject_IsTrue(text); + if (rc > 0) + return text; + Py_DECREF(text); if (rc < 0) return NULL; - if (rc) { - Py_INCREF(text); - return text; - } } else { rc = (it->sought_tag == Py_None); if (!rc) { - rc = PyObject_RichCompareBool(child->tag, + rc = PyObject_RichCompareBool(elem->tag, it->sought_tag, Py_EQ); - if (rc < 0) + if (rc < 0) { + Py_DECREF(elem); return NULL; + } } if (rc) { - Py_INCREF(child); - return (PyObject *)child; + return (PyObject *)elem; } + Py_DECREF(elem); } } else { PyObject *tail; - ParentLocator *next = it->parent_stack->next; + ParentLocator *next; if (it->gettext) { + Py_INCREF(cur_parent); tail = element_get_tail(cur_parent); - if (!tail) + if (!tail) { + Py_DECREF(cur_parent); return NULL; + } + Py_INCREF(tail); + Py_DECREF(cur_parent); } - else + else { tail = Py_None; - Py_XDECREF(it->parent_stack->parent); + Py_INCREF(tail); + } + next = it->parent_stack->next; + cur_parent = it->parent_stack->parent; PyObject_Free(it->parent_stack); it->parent_stack = next; + Py_XDECREF(cur_parent); /* Note that extra condition on it->parent_stack->parent here; * this is because itertext() is supposed to only return *inner* @@ -2179,12 +2203,14 @@ */ if (it->parent_stack->parent) { rc = PyObject_IsTrue(tail); + if (rc > 0) + return tail; + Py_DECREF(tail); if (rc < 0) return NULL; - if (rc) { - Py_INCREF(tail); - return tail; - } + } + else { + Py_DECREF(tail); } } }