ngx_resolver.c leaks memory?
is at rambler-co.ru
Thu Sep 17 15:45:16 MSD 2009
On Thu, Sep 17, 2009 at 02:40:23PM +0400, Maxim Dounin wrote:
> On Wed, Sep 16, 2009 at 09:24:33AM -0700, Matthew Dempsky wrote:
> > On Wed, Sep 16, 2009 at 3:35 AM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> > > Patch looks correct for me. It looks a bit fragile though,
> > > probably we need a bit more bulletproof code here.
> > Would you mind elaborating on what you think is fragile about it?
> > E.g., how would you rather this bug be patched?
> I personally prefer something that won't explode once "goto
> failed;" will be reused somewhere after first ngx_resolver_free(r,
> name->data). Just adding "name->data = NULL;" should be enough
> (though it's redundant right now).
No, the patch is good and I have commited it already.
> > > More generally - resolver known to leak, and probably requires
> > > code audit. It would be fine if you look into it. I believe
> > > Artem Bokhan will help with testing (cc'd as I'm not sure he is on
> > > English list).
> > Great. I'll spend some time looking at the rest of the code then.
> BTW, it looks like CNAME case in the same function will also leak
> as it sets rn->u.cname without freeing it first.
> Probably we just need something like allocation pools as used in
> other parts of nginx code. No idea why Igor wasn't used them in
> resolver. Igor, could you please explain reasons?
The pools can not be used here for several reasons:
1) there is DNS cache,
2) DNS request may be shared by several consumers.
However, I think reference counts should be added here.
More information about the nginx