httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <>
Subject Re: util_ldap [Bug 29217] - Remove references to calloc() and free()
Date Fri, 11 Jun 2004 16:33:35 GMT
The proper logic to add to a cache is


  test if exists again

  add element


because there is a race condition in the logic below


  test if the element exists

>> race is here, prior to wrlocking, another thread may wrlock->insert

  promote to wrlock


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
>Brad Nicholes
>Senior Software Engineer
>Novell, Inc., the leading provider of Net business solutions
>>>> Graham Leggett <> 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
>> 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
>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
>fix when I started trying to fix the code.

View raw message