[lxml-dev] Ref-counting bug returns in 1.3.1

[David M. Grimes]

I posted a patch for what I believed to be a reference-counting bug in 
Attrib.pop() based on the 1.3 release.  The patch was accepted, and is 
present in 1.3.1.  The patch is included at the end of this message.  
Looking through the generated C code, I'm no longer sure my patch was 
correct - perhaps just masking the underlying problem in 1.3.  I'm not 
fluent in Pyrex, so not sure if the python.Py_INCREF is really necessary 
for something which would be a "borrwed reference" in the C-API 
(PyTuple_GET_ITEM result).  It looks like the Pyrex "return" is 
generating it's own INCREF ...

Now, what is intriguing is that the 1.3.1 stock build is crashing again 
with the same symptom, and is easily reproducable with the following 
test program (this crashed after iteration  956 in i686 with python 2.4.4:

import lxml.etree as etree

xml = '''\
<?xml version="1.0"?>
<xml/>'''

for i in range(10000):
    print i
    et = etree.fromstring(xml)
    et.attrib.pop('x', None)

This dies at this point:

...
...
951
952
953
954
955
956
Fatal Python error: deallocating None
Aborted

Original 1.3 patch:

diff -urN lxml-1.3~/src/lxml/etree.pyx lxml-1.3/src/lxml/etree.pyx
--- lxml-1.3~/src/lxml/etree.pyx        2007-06-25 02:25:37.000000000 -0400
+++ lxml-1.3/src/lxml/etree.pyx 2007-06-27 15:36:15.000000000 -0400
@@ -1480,10 +1480,12 @@
             if python.PyTuple_GET_SIZE(default) == 0:
                 raise KeyError, key
             else:
-                return python.PyTuple_GET_ITEM(default, 0)
+                result = python.PyTuple_GET_ITEM(default, 0)
+                python.Py_INCREF(result)
         else:
             _delAttribute(self._element, key)
-            return result
+
+        return result

     def clear(self):
         cdef xmlNode* c_node



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://codespeak.net/pipermail/lxml-dev/attachments/20070703/c385c04e/attachment.htm