Seg fault: Possible race conditions in mod_mem_cache.c

[Jean-Jacques Clar]

Testing 2.0 and 2.1 Head. I am running a test that requires frequent ejections of cache entities: max_cache_size and max_object_count are smaller than my sampling. 2 threads running at the concurrently on 2 CPUs.   Thread1(T1): an entry is ejected from the cache in cache_insert(), resulting in a call to memcahe_cache_free() [c->free_entry(ejected)]. Refcount is 1, obj->cleanup is 0.   T2: a thread is calling apr_pool_clear() in worker_main/worker_thread() in the mpm main function (using worker|netware). That thread is calling decrement_refcount(), the registered cleanup function in mod_mem_cache.c. Both threads are working on the same cache_object. Refcount is 1, obj->cleanup is 0 when entering decrement_refcount().   Then there is a race in both functions where the atomic_dec in decrement_refcount() will change the refcount to 0 and then the if(obj->cleanup) will be successful, because it was set to 1 by T1 in the mean time. T1 and T2 will then call cleanup_cache_object() resulting in a double free on the same cache object.   There are probably a big battle happening between the 2 CPUs resulting in target memory bouncing on the bus between the 2 caches.   I tried the following patch, No seg fault but uncleaned memory when shutting down apache, maybe a small leak but not 100% sure about that. Will leave it running over the week end on my 4 procs box.   diff -u -r1.88.2.9 mod_mem_cache.c --- modules/experimental/mod_mem_cache.c 26 Aug 2004 16:59:44 -0000 1.88.2.9 +++ modules/experimental/mod_mem_cache.c 3 Sep 2004 20:50:36 -0000 @@ -156,12 +156,14 @@       * now. Increment the refcount before setting cleanup to avoid a race       * condition. A similar pattern is used in remove_url()       */ -    apr_atomic_inc(&obj->refcount); - -    obj->cleanup = 1; - -    if (!apr_atomic_dec(&obj->refcount)) { +    if(!apr_atomic_read(&obj->refcount)) {          cleanup_cache_object(obj); +    } +    else { +        apr_atomic_inc(&obj->refcount); +        obj->cleanup = 1; +        apr_atomic_dec(&obj->refcount);      }  }  /* Hope someone could come up with something better to fix the whole thing, I am out of time, I leaving and will bb only on Wednesday next week. Thanks and have a good long weekend. JJ