httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jean-Jacques Clar" <JJC...@novell.com>
Subject Seg fault: Possible race conditions in mod_mem_cache.c
Date Fri, 03 Sep 2004 21:02:19 GMT
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
 



Mime
View raw message