Re: squid-3.HEAD IPAddress leak

> Uhm, the kernel won't be free'ing userland memory at all; it doesn't give
> a rats how its managed.
>
> You're probably confused with the library - the libc is fumbling with
> all the struct addrinfo things.

Possibly. Once an external call is made I no longer care if its kernel or
library. Bad effects are equally dangerous to squid.

>
> Also, if the bug bugs you, create a temporary pointer and pass that in. ;)
>

Try it. You will be horrified.
The pointer-tree needs by the OS to work with screws that up. We would
need to allocate a new addrinfo for each master node and set its pointers
to new memory, copy the original data into it ... and hey! thats what
IPAddress::GetAddrInfo() does!

Do not confuse IPAddress::GetAddrInfo() which allocates an initializes a
'full' one-IP addrinfo* tree quickly which is about to be used in place of
a blocking getaddrinfo() system call.

The second-best alternative in many of these places is hard-coding
sockaddr_in* types and using #if USE_IPV6 to alternate the calls.

With IPAddress::InitAddrInfo() which allocates a single empty addrinfo*
ready for new data to be inserted and only sets the flags up.

Amos

>
>
> Adrian
>
> On Fri, Feb 08, 2008, Amos Jeffries wrote:
>> >
>> > On Thu, 2008-02-07 at 12:21 +0900, Adrian Chadd wrote:
>> >>
>> >> Well, I haven't removed the temporary malloc/free pair, whatever its
>> >> called;
>> >> I've just removed Amos' workaround in src/comm.cc so it doesn't leak
>> >> on my
>> >> system whilst I profile.
>> >>
>> >> Still, this is one of those "death of a thousand cuts" method of
>> >> killing performance..
>> >
>> > Right, I haven't seen the commit; care to mail the diff?
>> >
>>
>>
>> Here you go.
>> "
>> --- src/comm.cc 2008-01-19 20:50:42.000000000 +1300
>> +++ src/comm.cc-2 2008-02-08 09:56:11.000000000 +1300
>> @@ -1381,7 +1381,7 @@
>>
>> }
>>
>> -#ifdef _SQUID_LINUX_
>> +#ifdef 0 // _SQUID_LINUX_
>> /* 2007-11-27:
>> * Linux Debian replaces our allocated AI pointer with garbage when
>> * connect() fails. This leads to segmentation faults deallocating
>> "
>>
>>
>> In reply to your last few emails Adrian.
>>
>> re: can we join up on IRC?
>>
>> I may not be able to for the next few days, too much else on.
>>
>>
>> re: Why are you trying to allocate the structure on invocation of
>> GetAddrInfo() ?
>>
>> The design was to follow the well-known structure of the kernel call
>> getaddrinfo(), which fills kernel-managed memory in a thread-safe way
>> down a dynamic structure. The addrinfo structs are too nasty to use
>> naked for anything like the amount of comm usage squid has. They are not
>> an object per-se, but the root of a pointer tree to various types of
>> node, which depend on the flags for their memory sizes.
>> We could make a variant of the call which takes an addrinfo object and
>> sets it up to point at the IPAddress internals correctly. BUT the bug we
>> are looking at would then cause the IPAddress to become garbage, maybe
>> have the kernel free'ing stack memory, etc.
>>
>>
>> re: Argh, this temporary malloc/free pair is peppered throughout the
>> codebase! Grr.
>>
>> You already know there are naked comm calls everywhere. :-( Most of
>> them
>> calls addrinfo was desgined for use in.
>>
>>
>> re: I've removed that hack, and things work fine for me. Ubuntu 7.01
>> here,
>> x86_64.
>>
>> Yes, I found no problem on ubuntu also. I simply could not (after very
>> little searching) find a #if test that would only work for Debian.
>>
>>
>> Amos
>>
>
> --
> - Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial Squid
> Support -
> - $25/pm entry-level VPSes w/ capped bandwidth charges available in WA -
>
Received on Thu Feb 07 2008 - 14:59:53 MST