Commit ebf37a2f authored by Eli Bendersky's avatar Eli Bendersky

Fixes and enhancements to _elementtree:

* Fixed refleak problems when GC collection is run (see messages in
  issue #14065)
* Added weakref support to Element objects
parent 5c73e8ea
...@@ -1859,6 +1859,41 @@ class BasicElementTest(unittest.TestCase): ...@@ -1859,6 +1859,41 @@ class BasicElementTest(unittest.TestCase):
gc_collect() gc_collect()
self.assertIsNone(wref()) self.assertIsNone(wref())
# A longer cycle: d->e->e2->d
e = ET.Element('joe')
d = Dummy()
d.dummyref = e
wref = weakref.ref(d)
e2 = ET.SubElement(e, 'foo', attr=d)
del d, e, e2
gc_collect()
self.assertIsNone(wref())
# A cycle between Element objects as children of one another
# e1->e2->e3->e1
e1 = ET.Element('e1')
e2 = ET.Element('e2')
e3 = ET.Element('e3')
e1.append(e2)
e2.append(e2)
e3.append(e1)
wref = weakref.ref(e1)
del e1, e2, e3
gc_collect()
self.assertIsNone(wref())
def test_weakref(self):
flag = False
def wref_cb(w):
nonlocal flag
flag = True
e = ET.Element('e')
wref = weakref.ref(e, wref_cb)
self.assertEqual(wref().tag, 'e')
del e
self.assertEqual(flag, True)
self.assertEqual(wref(), None)
class ElementTreeTest(unittest.TestCase): class ElementTreeTest(unittest.TestCase):
def test_istype(self): def test_istype(self):
......
...@@ -48,6 +48,7 @@ ...@@ -48,6 +48,7 @@
/* See http://www.python.org/psf/license for licensing details. */ /* See http://www.python.org/psf/license for licensing details. */
#include "Python.h" #include "Python.h"
#include "structmember.h"
#define VERSION "1.0.6" #define VERSION "1.0.6"
...@@ -229,6 +230,8 @@ typedef struct { ...@@ -229,6 +230,8 @@ typedef struct {
ElementObjectExtra* extra; ElementObjectExtra* extra;
PyObject *weakreflist; /* For tp_weaklistoffset */
} ElementObject; } ElementObject;
static PyTypeObject Element_Type; static PyTypeObject Element_Type;
...@@ -261,17 +264,24 @@ create_extra(ElementObject* self, PyObject* attrib) ...@@ -261,17 +264,24 @@ create_extra(ElementObject* self, PyObject* attrib)
LOCAL(void) LOCAL(void)
dealloc_extra(ElementObject* self) dealloc_extra(ElementObject* self)
{ {
int i; if (!self->extra)
return;
/* Avoid DECREFs calling into this code again (cycles, etc.)
*/
ElementObjectExtra *myextra = self->extra;
self->extra = NULL;
Py_DECREF(self->extra->attrib); Py_DECREF(myextra->attrib);
for (i = 0; i < self->extra->length; i++) int i;
Py_DECREF(self->extra->children[i]); for (i = 0; i < myextra->length; i++)
Py_DECREF(myextra->children[i]);
if (self->extra->children != self->extra->_children) if (myextra->children != myextra->_children)
PyObject_Free(self->extra->children); PyObject_Free(myextra->children);
PyObject_Free(self->extra); PyObject_Free(myextra);
} }
/* Convenience internal function to create new Element objects with the given /* Convenience internal function to create new Element objects with the given
...@@ -308,6 +318,8 @@ create_new_element(PyObject* tag, PyObject* attrib) ...@@ -308,6 +318,8 @@ create_new_element(PyObject* tag, PyObject* attrib)
Py_INCREF(Py_None); Py_INCREF(Py_None);
self->tail = Py_None; self->tail = Py_None;
self->weakreflist = NULL;
ALLOC(sizeof(ElementObject), "create element"); ALLOC(sizeof(ElementObject), "create element");
PyObject_GC_Track(self); PyObject_GC_Track(self);
return (PyObject*) self; return (PyObject*) self;
...@@ -328,6 +340,7 @@ element_new(PyTypeObject *type, PyObject *args, PyObject *kwds) ...@@ -328,6 +340,7 @@ element_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
e->tail = Py_None; e->tail = Py_None;
e->extra = NULL; e->extra = NULL;
e->weakreflist = NULL;
} }
return (PyObject *)e; return (PyObject *)e;
} }
...@@ -576,19 +589,28 @@ element_gc_traverse(ElementObject *self, visitproc visit, void *arg) ...@@ -576,19 +589,28 @@ element_gc_traverse(ElementObject *self, visitproc visit, void *arg)
static int static int
element_gc_clear(ElementObject *self) element_gc_clear(ElementObject *self)
{ {
PyObject *text = JOIN_OBJ(self->text);
PyObject *tail = JOIN_OBJ(self->tail);
Py_CLEAR(self->tag); Py_CLEAR(self->tag);
Py_CLEAR(text);
Py_CLEAR(tail);
/* After dropping all references from extra, it's no longer valid anyway, /* The following is like Py_CLEAR for self->text and self->tail, but
** so fully deallocate it (see also element_clearmethod) * written explicitily because the real pointers hide behind access
* macros.
*/ */
if (self->extra) { if (self->text) {
dealloc_extra(self); PyObject *tmp = JOIN_OBJ(self->text);
self->extra = NULL; self->text = NULL;
Py_DECREF(tmp);
}
if (self->tail) {
PyObject *tmp = JOIN_OBJ(self->tail);
self->tail = NULL;
Py_DECREF(tmp);
} }
/* After dropping all references from extra, it's no longer valid anyway,
* so fully deallocate it.
*/
dealloc_extra(self);
return 0; return 0;
} }
...@@ -596,6 +618,10 @@ static void ...@@ -596,6 +618,10 @@ static void
element_dealloc(ElementObject* self) element_dealloc(ElementObject* self)
{ {
PyObject_GC_UnTrack(self); PyObject_GC_UnTrack(self);
if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);
/* element_gc_clear clears all references and deallocates extra /* element_gc_clear clears all references and deallocates extra
*/ */
element_gc_clear(self); element_gc_clear(self);
...@@ -626,10 +652,7 @@ element_clearmethod(ElementObject* self, PyObject* args) ...@@ -626,10 +652,7 @@ element_clearmethod(ElementObject* self, PyObject* args)
if (!PyArg_ParseTuple(args, ":clear")) if (!PyArg_ParseTuple(args, ":clear"))
return NULL; return NULL;
if (self->extra) { dealloc_extra(self);
dealloc_extra(self);
self->extra = NULL;
}
Py_INCREF(Py_None); Py_INCREF(Py_None);
Py_DECREF(JOIN_OBJ(self->text)); Py_DECREF(JOIN_OBJ(self->text));
...@@ -1693,7 +1716,7 @@ static PyTypeObject Element_Type = { ...@@ -1693,7 +1716,7 @@ static PyTypeObject Element_Type = {
(traverseproc)element_gc_traverse, /* tp_traverse */ (traverseproc)element_gc_traverse, /* tp_traverse */
(inquiry)element_gc_clear, /* tp_clear */ (inquiry)element_gc_clear, /* tp_clear */
0, /* tp_richcompare */ 0, /* tp_richcompare */
0, /* tp_weaklistoffset */ offsetof(ElementObject, weakreflist), /* tp_weaklistoffset */
0, /* tp_iter */ 0, /* tp_iter */
0, /* tp_iternext */ 0, /* tp_iternext */
element_methods, /* tp_methods */ element_methods, /* tp_methods */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment