[ldns-users] pyldns - memory leaks and double freeing

[Willem Toorop]

Hi Bedrich,

I've applied the patch in trunk.
Thanks!

Willem

Op 04-05-11 09:26, Bedrich Kosata schreef:
> Hi Willem,
>
> by looking at the current SVN code, I found out, that I actually did
not include part of the code into the patch I sent last time.
> This is the part about RRSIG checking - marked in my last post below.
> I am attaching the patch for ldns_rr.i now.
>
> Best regards
>
> Beda
>
> p.s.- I am currently working on some more memory issues related to rr
and rdf sharing the same data. I hope to be able to send something soon.
>
>
> On 04/13/2011 04:54 PM, Willem Toorop wrote:
> Hi Bedrich,
>
> I've applied your patches in trunk.
> The tests run all fine with me.
> Thanks for fixing this.
>
> Best regards, Willem
>
>
> On 04/13/11 08:45, Bedrich Kosata wrote:
> >>> Hi again,
> >>>
> >>> as nobody seemed to have anything against my proposal, I prepared the
> >>> patches that should fix the problems I outlined before :)
> >>>
> >>> When the patches are applied, the python bindings should always have
> >>> rr_list and rr structs separated in memory. This is accomplished by
> >>> always cloning rr data when either adding them to an rr_list or
> >>> retrieving them from the rr_list.
>
> > This was not in the last patch..
>
> >>> Most of the changes should be transparent and should not require
> >>> modifications to existing code. The only outside change I had to make
> >>> was in the "ldns_verify_rrsig_keylist_notime" and
> >>> "ldns_verify_rrsig_keylist" functions, because it copies pointers
> to the
> >>> correct keys into a second rr_list. I solved this by replacing each of
> >>> these functions by two new ones (thus creating four functions). All
> >>> versions take only 3 arguments and differ in the way they treat the
> >>> good_keys value. One versions name ends with "_status_only" (e.g.
> >>> ldns_verify_rrsig_keylist_notime_status_only) and this does not report
> >>> the good_keys at all, just the result of verification. The other ends
> >>> with "_" (e.g. ldns_verify_rrsig_keylist_notime_) and returns a tuple
> >>> (status, good_keys) where good_keys are indexes of the keys in the key
> >>> list supplied to the function. Thus the whole business of two rr_lists
> >>> sharing the same data is sidestepped.
> >>>
>
> > .. and here it ends
>
> >>> The patches are made for each file separately because the fixes are
> >>> mostly independent. I also included several test scripts which should
> >>> show how the situation was fixed. They either show a situation
> where the
> >>> old version would crash with a segmentation fault or display the
> amount
> >>> of memory the app has used thus pointing to a memory leak. When used
> >>> with patched sources, none of the scripts should crash and none should
> >>> report more that ~30MB of memory used.
> >>>
> >>> Best regards
> >>>
> >>> Beda
> >>>
> >>> p.s.- the patch for ldns_rdf.i is in fact not related to the rest and
> >>> fixes and unrelated memory leak in ldns_rdf.new_frm_str
> >>>
> >>> p.p.s.- I did not touch a related issue of rr and rdf relationship. In
> >>> current version, the rdf data does not live beyond the lifetime of the
> >>> corresponding rr. This is demonstrated in the attached
> >>> test-rrlist-get-rdf.py. The problem could again be solved by
> cloning all
> >>> rdfs on retrieval and addition from/into an rr.
> >>> However, I will wait how the current patch is received before I start
> >>> thinking about fixing it.
> >>>
> >>>
> >>> On 03/25/2011 09:48 AM, Bedrich Kosata wrote:
> >>>> Hi everybody,
> >>>>
> >>>> while trying to find a cause of a memory leak in a simple script, I
> >>>> found a nest of memory related issues in the python bindings.
> >>>> The problems are all related to one common problem - who takes
> care of
> >>>> memory of composite objects, such as ldns_rr_lists or ldns_pkt.
> >>>> For example, in the current version, ldns_pkt bindings use
> ldns_pkt_free
> >>>> to free a packet structure, however, when a rr_list is taken from the
> >>>> packet and returned from a function, the packet gets out of scope, is
> >>>> freed and the rr_list refers to already freed memory.
> >>>> On the other hand, a rr_list only frees its own memory, not memory of
> >>>> the stored rrs. This can lead to memory leaks.
> >>>> I am attaching two scripts that demonstrate these problems (it
> might be
> >>>> necessary to have the sources patched with the "freeing None" patch I
> >>>> sent last week).
> >>>> I would be willing to have a stab at the problems (provided I get
> >>>> clearance from my boss :)), but the only solution I think would
> be clean
> >>>> enough, is to clone the necessary bits where needed. This might
> lead to
> >>>> some inefficiency and slowdowns (probably not big), so I would
> like to
> >>>> ask if this is ok.
> >>>> If there is anyone else willing to fix this, I would be happy to
> act as
> >>>> a tester.
> >>>>
> >>>> Cheers
> >>>>
> >>>> Beda
> >>>>
> >>>> p.s.- test-pkt-free.py crashes with a segmentation fault,
> >>>> test-rr-list.py ends up eating about 130 MB of memory and more than
> >>>> 400000 python objects which python cannot free.
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> 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
>

> _______________________________________________
> ldns-users mailing list
> ldns-users at open.nlnetlabs.nl
> http://open.nlnetlabs.nl/mailman/listinfo/ldns-users


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.nlnetlabs.nl/pipermail/ldns-users/attachments/20110517/baaea008/attachment.htm>