bpo-31499, xml.etree: Fix xmlparser_gc_clear() crash #3641
Conversation
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 <[email protected]>
Modules/_elementtree.c
Outdated
| @@ -3411,7 +3411,10 @@ xmlparser_gc_traverse(XMLParserObject *self, visitproc visit, void *arg) | |||
| static int | |||
| xmlparser_gc_clear(XMLParserObject *self) | |||
| { | |||
| EXPAT(ParserFree)(self->parser); | |||
| if (self->parser != NULL) { | |||
| EXPAT(ParserFree)(self->parser); | |||
There was a problem hiding this comment.
Perhaps it would be safer to set self->parser = NULL before calling ParserFree.
There was a problem hiding this comment.
IMHO it's overkill. EXPAT(ParserFree) calls a C function which is outside the "Python" world, I don't see how it could trigger a reentrant call.
There was a problem hiding this comment.
Well, I made the change anyway. It doesn't harm much :-)
| @@ -0,0 +1,3 @@ | |||
| xml.etree: xmlparser_gc_clear() now sets self.parser to NULL to prevent a | |||
There was a problem hiding this comment.
Too much implementation details. They are good on the tracker or in commit message, but not in a changelog. Look at this from the point of view of common Python user.
There was a problem hiding this comment.
Done, I simplified the NEWS entry.
|
Thanks for your review @serhiy-storchaka! |
|
Thanks @Haypo for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6. |
…3641) * 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 <[email protected]> (cherry picked from commit e727d41)
|
GH-3645 is a backport of this pull request to the 3.6 branch. |
…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 <[email protected]> (cherry picked from commit e727d41)
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.
https://bugs.python.org/issue31499