Memory leak parsing/relaying from detail files?

[Nathan Ward]

> On 16/04/2016, at 03:55, Alan DeKok <aland at deployingradius.com> wrote:
> 
> On Apr 14, 2016, at 8:43 PM, Nathan Ward <lists+freeradius at daork.net> wrote:
> 
>> However, I have since noted that the memory usage on the process still increases over time, at the same rate as before.
>> I think what is happening is that now the that the talloc context is set, valgrind no longer recognises it as a leak as it doesn’t consider memory that talloc is managing to be a leak, however we are still allocating memory for something that is never used again (the Attr-95 attribute name) and expanding that talloc context (I don’t know much about talloc, can you tell? :-).
> 
>  Are you sure it's the same issue?
> 
>  i.e. if you update the dictionary to have NAS-IPv6-Address be of type "octets", does the same file now *not* leak memory?


I’ll do some further testing shortly, but so far I have set NAS-IPv6-Address to octets *and* removed it before writing to the file which does not leak memory.
I will try it without removing it, but I don’t expect it will make a difference - let’s find out.

>> What did you think of my prior suggestion in question 2 of my original email:
>> 2) I would like to figure out this memory leak myself and submit a patch, but I’m not sure how this part of the code works. I *think* that this should be freed after being proxied to the backend server - so, perhaps in process.c somewhere after a packet is sent, it should have any vp das that have is_unknown set to true freed? I’m not clear what impact that would have on other bits of code.
> 
>  The memory should be freed automatically.
> 
>  I've pushed some changes which may help.  The issue is that we have a talloc context.  When the context gets freed, everything in it gets freed.  The context includes a VALUE_PAIR, usually 'vp', and the VALUE_PAIR includes a DICT_ATTR, vp->da.  The da (if unknown) SHOULD be parented from the vp.  So when the vp is freed, so is the da.

OK, I’ll go read some code and try understand this some more.

>> Of course, happy to write the code, but some guidance on whether this is appropriate or likely to harm other processes would be helpful.
> 
>  The key is to ensure that the da has the correct parent.  It will then be automatically freed as necessary.

Got it.

>> Naturally, I’m progressing this with the vendor for a fix on their end, too.
> 
>  That would be good.
> 
>  I'll see if there are fixes to the detail file reader which may help.  But the patches I pushed today might fix it...

Thanks.

I’ll do the above test, and have a look at the recent patches and see if they help, and then report back.

--
Nathan Ward