classification
Title: Element should support cyclic GC
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eli.bendersky Nosy List: eli.bendersky, flox, jcea, loewis, python-dev, skrah
Priority: release blocker Keywords: patch

Created on 2012-02-20 16:26 by loewis, last changed 2012-04-04 13:12 by eli.bendersky. This issue is now closed.

Files
File name Uploaded Description Edit
a.py loewis, 2012-02-20 16:26
issue14065.1.patch eli.bendersky, 2012-03-24 16:48 review
issue14065_buildfix.patch eli.bendersky, 2012-04-04 05:13 review
Messages (11)
msg153784 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-02-20 16:26
The C implementation of xml.etree.ElementTree.Element needs to support cyclic GC. The attached script demonstrates the lack to support that: in 3.2, the script passes; in 3.3 (7697223df6df) it fails with an AssertionError as the cycle was not cleared. 

This is an incompatible change from 3.2.
msg155991 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-03-16 04:54
Martin, why do you think it's important for Element to support this? After all, this is XML, not an arbitrary tree. As such, the children of Element can only be other elements, and attribute values should be strings. Anything else will result in errors when attempting to write that Element into a real XML file. Semantically it doesn't make sense for the value of an attribute to be a list or any other container, for that matter.

In your sample code, if you attempt to dump or write L[0] before deleting it, you'll get an error.

Adding GC handling complicates the code (even if not by too much), and this complication should be justified. Can you see a valid use case where GC handling would be required?
msg156151 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-03-17 10:25
As a matter of principle, garbage collection in Python should *always* work, for all types, except for the one documented exception (cycles involving __del__). Failure of a type to properly garbage collect should be considered as serious as an interpreter crash; I hence propose this issue as release blocker.

In addition, failure to support tp_traverse means that gc.get_referents doesn't work for the type, which is an inconvenience even in regular (non-cyclic) usage.
msg156706 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-03-24 16:48
Find attached a patch.

Added cyclic GC support to Element objects. Also added tests that verify that cycles involving Element objects are being collected.

I'd really appreciate a review on this, since this is the first time I have to explicitly deal with cyclic GC from C extensions.
msg157136 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-03-30 13:39
New changeset 0ca32013d77e by Eli Bendersky in branch 'default':
Issue #14065: Added cyclic GC support to ET.Element
http://hg.python.org/cpython/rev/0ca32013d77e
msg157296 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-04-01 14:43
Re-opening, since GC collection of length-2 cycles cause refleaks (Issue #14464).

For now the test was reverted in changeset c5cf48752d81 - it has to be put back when this is fixed.
msg157439 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-04-03 19:04
New changeset 14abfa27ff19 by Eli Bendersky in branch 'default':
Fixes and enhancements to _elementtree:
http://hg.python.org/cpython/rev/14abfa27ff19
msg157448 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-04-03 23:45
Just in case you missed it: The Windows buildbots fail to compile
14abfa27ff19:

http://www.python.org/dev/buildbot/all/builders/x86%20Windows7%203.x
msg157459 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-04-04 04:57
Stefan, thanks. The windows bots were down when I was looking :-/

I'll work on a fix
msg157460 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-04-04 05:13
Attaching a patch that should fix the build - I don't have write access to commit where I am - will be able to commit it later today.
msg157475 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-04-04 13:12
Fix committed - Windows bots now compile successfully.
History
Date User Action Args
2012-04-04 13:12:31eli.benderskysetmessages: + msg157475
2012-04-04 05:13:07eli.benderskysetfiles: + issue14065_buildfix.patch

messages: + msg157460
2012-04-04 04:57:06eli.benderskysetmessages: + msg157459
2012-04-03 23:45:24skrahsetnosy: + skrah
messages: + msg157448
2012-04-03 19:04:55eli.benderskysetstatus: open -> closed
resolution: fixed
stage: needs patch -> resolved
2012-04-03 19:04:30python-devsetmessages: + msg157439
2012-04-01 14:44:09eli.benderskylinkissue14464 superseder
2012-04-01 14:43:22eli.benderskysetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg157296

stage: resolved -> needs patch
2012-03-30 13:40:47eli.benderskysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-03-30 13:39:52python-devsetnosy: + python-dev
messages: + msg157136
2012-03-24 16:48:12eli.benderskysetfiles: + issue14065.1.patch
keywords: + patch
messages: + msg156706

stage: needs patch -> patch review
2012-03-17 10:25:30loewissetpriority: normal -> release blocker

messages: + msg156151
2012-03-16 04:54:05eli.benderskysetmessages: + msg155991
2012-03-16 04:03:20eli.benderskysetassignee: eli.bendersky
versions: - Python 3.2
2012-03-03 07:28:01eli.benderskysetnosy: + eli.bendersky
2012-02-20 16:33:43pitrousetversions: + Python 3.2
nosy: + flox

components: + Library (Lib)
type: behavior
stage: needs patch
2012-02-20 16:32:21jceasetnosy: + jcea
2012-02-20 16:26:47loewiscreate