httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Ames <grega...@remulak.net>
Subject Re: [PATCH2] Re: Seg fault: race conditions in mod_mem_cache.c
Date Wed, 15 Sep 2004 18:41:14 GMT
Jean-Jacques Clar wrote:
> What about calling memcache_cache_free() instead of the
> apr_atomic_set(). The refcount must be more than 1, we
> hold the mutex. It should work just fine..
> What do you think?

That looks better as far as not loosing an update from an unlocked thread.

I'm still trying to learn the complete design though.  I see that 
memcache_cache_free() can return with OBJECT_CLEANUP_BIT set, and that 
decrement_refcount() will react to it and free the memory if it is registered as 
a cleanup.  Will decrement_refcount() always be registered?  Just wondering if 
there are any code paths where we might leak memory.

There's a similar race in remove_entity() using apr_atomic_set() to turn on the 
cleanup bit:

@@ -536,10 +529,10 @@
       * an object marked for cleanup is by design not in the
       * hash table.
       */
-    if (!obj->cleanup) {
-        cache_remove(sconf->cache_cache, obj);
-        obj->cleanup = 1;
-        ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, "gcing a cache entry");
+     if (!(apr_atomic_read(&obj->refcount) & OBJECT_CLEANUP_BIT)) {
+         cache_remove(sconf->cache_cache, obj);
+         apr_atomic_set(&obj->refcount, obj->refcount | OBJECT_CLEANUP_BIT);
+         ap_log_error(APLOG_MARK, APLOG_INFO, 0, NULL, "gcing a cache entry");
      }

Greg


Mime
View raw message