The proper logic to add to a cache is
wrlock
test if exists again
add element
unlock
because there is a race condition in the logic below
rdlock
test if the element exists
>> race is here, prior to wrlocking, another thread may wrlock->insert
promote to wrlock
insert
unlock
At 06:07 PM 6/10/2004, Brad Nicholes wrote:
> 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
>--
