[ldns-users] contrib/python: memory leak in ldns_pkt.new_query

[Johannes Naab]

Hi,

the patch works well for me. Thank you for the fast response, the patch
and the explanation.

Johannes

On 08/07/2013 02:47 PM, Karel Slany wrote:
> Hello,
> 
> the patch in the attachment solves the memory leak and the owner problem.
> 
> Someone at NLnet Labs, please, apply the patch onto the ldns svn.
> 
> 
> Using '%newobject ldns_pkt_query_new;' solved only one half of the
> problem. Dname has to be cloned before being used in new_query because
> ldns_pkt_query_new() uses the rr_name pointer directly instead of
> creating a copy. Double free is the result as the same rr_name object is
> being destroyed once in dname and second in pkt.
> 
> The patch creates a new wrapper function that automatically clones the
> dname rr_name before calling ldns_pkt_query_new().
> 
> Best regards,
> K.
> 
> Am 07.08.2013 11:26, schrieb Karel Slany:
>> Hello Johannes,
>>
>> I'll have look at it.
>>
>> K.
>>
>> Am 06.08.2013 19:59, schrieb Johannes Naab:
>>> Hi,
>>>
>>> The python wrapper for ldns appears to leak memory when packets are
>>> created by ldns_pkt.new_query.
>>>
>>> The following is a short example (tested in ldns-1.6.16, Python 2.7.5
>>> Linux 3.10, amd64):
>>> #!/usr/bin/env python2.7
>>> import ldns
>>>
>>> dname = ldns.ldns_dname("test.nic.cz")
>>>
>>> def leak():
>>>     pkt = ldns.ldns_pkt.new_query(dname, ldns.LDNS_RR_TYPE_A,
>>> ldns.LDNS_RR_CLASS_IN, 0)
>>>
>>> while True:
>>>     leak()
>>>
>>>
>>> Root of the problem seems to be, that the pointer to the ldns_pkt struct
>>> is not considered owned by the swig wrapper, and thus the pkt struct is
>>> not freed upon garbage collection in python.
>>>
>>> import ldns
>>> dname = ldns.ldns_dname("test.nic.cz")
>>> pkt = ldns.ldns_pkt.new_query(dname, ldns.LDNS_RR_TYPE_A,
>>>                               ldns.LDNS_RR_CLASS_IN, 0)
>>> print pkt.thisown
>>>
>>> => False.
>>>
>>> If new_query is replaced by new_query_frm_str the packet is "owned" by
>>> the swig wrapper, and the packet does not leak.
>>>
>>>
>>> pkt = ldns.ldns_pkt.new_query_frm_str(str(dname), ldns.LDNS_RR_TYPE_A,
>>>                                       ldns.LDNS_RR_CLASS_IN, 0)
>>> print pkt.thisown
>>>
>>> => True
>>>
>>> I tried to convince swig that the struct should be owned by the wrapper
>>> by inserting a "%newobject ldns_pkt_query_new;" below line 44 in
>>> contrib/python/ldns_packet.i. This made swig own the pointer, but
>>> freeing the packet on garbage collection made python crash (memory
>>> corruption, double free). My knowledge and understanding of swig is limited.
>>>
>>>
>>> Johannes
>>> _______________________________________________
>>> ldns-users mailing list
>>> ldns-users at open.nlnetlabs.nl
>>> http://open.nlnetlabs.nl/mailman/listinfo/ldns-users
>>>
>>
>> _______________________________________________
>> ldns-users mailing list
>> ldns-users at open.nlnetlabs.nl
>> http://open.nlnetlabs.nl/mailman/listinfo/ldns-users
>>
> 


--