It appears to me that if it doesn't handle low memory situations or
it is giving false positives, those are separate issues from pools vs.
calloc/free. I still think we need to implement some better monitoring
or logging code in the cache_mgr and enhance the cache-status pages so
that we can track things like false positives. Maybe tracking the
entries by user name and authentication state rather than just the
number entries and how often the cache was hit.
Maybe the real problem is with the locking. In fact just taking a
quick scan through the code again, I am seeing something that bothers me
in util_ldap_cache_comparedn()
if (curl) {
/* compare successful - add to the compare cache */
LDAP_CACHE_RDLOCK();
newnode.reqdn = (char *)reqdn;
newnode.dn = (char *)dn;
util_ald_cache_insert(curl->dn_compare_cache, &newnode);
LDAP_CACHE_UNLOCK();
}
It appears to be acquiring a read lock but then inserts a new node into
the cache. Shouldn't it be acquiring a write lock before doing an
insert?
Brad
Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com
>>> Graham Leggett <[EMAIL PROTECTED]> Thursday, June 10, 2004 4:24:54
PM >>>
Brad Nicholes wrote:
> Do we even know that there is a problem with this code? I haven't
seen
> any memory leaks so far. I would hate to go to all of the work to
> redesign and rewrite the ldap_cache manager for little to no gain.
It does not seem to handle the "we ran out of memory while trying to
add
to the cache" case very gracefully. It doesn't crash any more, but I'm
experiencing false negatives still, which is the problem I was trying
to
fix when I started trying to fix the code.
Regards,
Graham
--
