Brad Nicholes wrote:
I have considered using a pool per entry in other caching code that I have written just so that I could have much finer control over allocating and freeing the memory. But in the end what it really comes down to is malloc and free and if that is what you really want, then why not just use malloc and free.
The LDAP cache is a complex memory structure - each cache entry consists of a struct with a whole lot of malloc'ed blocks. And there are different kind of cache entries (bind cache, opcache). Deleting an entry means walking through the struct and relying on a human writing code to delete each block allocated one by one, and finally to delete the struct. If one of the free()'s are missed, leak.
If there was a single pool per cache entry, to remove the entry, just call apr_pool_destroy() once, not free() many times, and be confident that the memory is gone and leak free.
Of course if we're forced to do this the manual way because it's done manually by the shared memory case, then I can see that having two methods of cleanup (one for shared, and one for unshared) is unnecessarily complex.
> Pools just add a layer of memory
management between your application and the actual allocation that may be of no use. You have simply replaced malloc() with apr_pool_create() and free() with apr_pool_destroy(). If you forget to call apr_pool_destroy(), you have a memory leak in the same way as you would if you forget to call free().
There is not a one-to-one replacement, no. apr_pool_destroy() would be called once for cache entry delete, instead of free() many times for each cache entry attribute.
In addition, memory is freed continuously as cache entries are expired and replaced with new ones, not when the process is cleaned up. The cache entries are reused, but the cache attributes are not - they're malloc'ed and (hopefully) free'd.
I think this hinges on whether it is possible to have memory pools with a shared memory backend. If it is, then shared memory and unshared can use the same allocate and cleanup structure.
Outside of some debugging help during development, converting to pools and reslists is just adding a lot of unneeded overhead.
Overhead (within reason) is cheap compared to an unstable server, which is what mod_*ldap currently is in my install. :(
Regards, Graham --
