Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.3k
gh-146056: Fix TreeBuilder stack in xml.etree#146062
gh-146056: Fix TreeBuilder stack in xml.etree#146062vstinner wants to merge 1 commit intopython:mainfrom
Conversation
No longer create a stack of 20 items, but create an empty stackinstead. It prevents crashes when the stack list is discovered bygc.get_referrers() or other functions.Fix also reference counting in treebuilder_handle_end().
vstinner commentedMar 17, 2026
I added "skip news" since this issue requires calling |
| item = self->last; | ||
| self->last = Py_NewRef(self->this); | ||
| Py_XSETREF(self->last_for_tail, self->last); | ||
| Py_XSETREF(self->last_for_tail,Py_NewRef(self->last)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is unrelated.
If rewrite this code to be more explicit or safe, I would write something like
PyObject*last=self->last;PyObject*last_for_tail=self->last_for_tail;PyObject*this=self->this;self->index--;self->this=Py_NewRef(PyList_GET_ITEM(self->stack,self->index));self->last=Py_NewRef(this);self->last_for_tail=Py_NewRef(this);Py_DECREF(last);Py_XDECREF(last_for_tail);if (treebuilder_append_event(self,self->end_event_obj,this)<0) {Py_DECREF(this);returnNULL; }returnthis;
But we should also look a the other ends -- how these attributes are set in other code in this file. This is a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I wrote#146167 which uses your suggestion.
| t->comment_factory = NULL; | ||
| t->pi_factory = NULL; | ||
| t->stack = PyList_New(20); | ||
| t->stack = PyList_New(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This can have performance impact.
vstinner commentedMar 19, 2026
I abandon my change since a more generic change was merged:#146129. |
Uh oh!
There was an error while loading.Please reload this page.
No longer create a stack of 20 items, but create an empty stack instead. It prevents crashes when the stack list is discovered by gc.get_referrers() or other functions.
Fix also reference counting in treebuilder_handle_end().