> 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 - >
