What specific problem is this patch correcting? I committed a fix for 21287 prior to the holidays. The idea behind using atomic operators on refcount is to avoid the need to acquire the mutex when incrementing/decrementing refcount.
Bill
[EMAIL PROTECTED] wrote:
clar 2004/01/07 11:35:16
Modified: modules/experimental mod_mem_cache.c
Log:
Fix for Bug 21287.
Patch extracted and modified from attachment in Bug 21285.
The submitted patch was combining fix for 21285 and 21287.
It is extending the mutex protection in decrement_refcount() and remove_url().
Revision Changes Path
1.103 +9 -26 httpd-2.0/modules/experimental/mod_mem_cache.c
Index: mod_mem_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
retrieving revision 1.102
retrieving revision 1.103
diff -u -r1.102 -r1.103
--- mod_mem_cache.c 6 Jan 2004 21:57:50 -0000 1.102
+++ mod_mem_cache.c 7 Jan 2004 19:35:16 -0000 1.103
@@ -348,33 +348,26 @@
cache_remove(sconf->cache_cache, obj);
obj->cleanup = 1;
}
- if (sconf->lock) {
- apr_thread_mutex_unlock(sconf->lock);
- }
+ }
+ else if (sconf->lock) {
+ apr_thread_mutex_lock(sconf->lock);
}
/* Cleanup the cache object */
#ifdef USE_ATOMICS
- if (!apr_atomic_dec32(&obj->refcount)) {
- if (obj->cleanup) {
- cleanup_cache_object(obj);
- }
- }
+ if (!apr_atomic_dec32(&obj->refcount) && (obj->cleanup)) {
#else
- if (sconf->lock) {
- apr_thread_mutex_lock(sconf->lock);
- }
obj->refcount--;
/* If the object is marked for cleanup and the refcount
* has dropped to zero, cleanup the object
*/
if ((obj->cleanup) && (!obj->refcount)) {
+#endif
cleanup_cache_object(obj);
}
if (sconf->lock) {
apr_thread_mutex_unlock(sconf->lock);
}
-#endif
return APR_SUCCESS;
}
static apr_status_t cleanup_cache_mem(void *sconfv)
@@ -739,35 +732,25 @@
obj = cache_find(sconf->cache_cache, key); if (obj) {
- mem_cache_object_t *mobj;
cache_remove(sconf->cache_cache, obj);
- mobj = (mem_cache_object_t *) obj->vobj;
#ifdef USE_ATOMICS
/* Refcount increment in this case MUST be made under * protection of the lock */
apr_atomic_inc32(&obj->refcount);
+ obj->cleanup = 1;
+ if (!apr_atomic_dec32(&obj->refcount)) {
#else
+ obj->cleanup = 1;
if (!obj->refcount) {
- cleanup_cache_object(obj);
- obj = NULL;
- }
#endif
- if (obj) {
- obj->cleanup = 1;
+ cleanup_cache_object(obj);
}
}
if (sconf->lock) {
apr_thread_mutex_unlock(sconf->lock);
}
-#ifdef USE_ATOMICS
- if (obj) {
- if (!apr_atomic_dec32(&obj->refcount)) {
- cleanup_cache_object(obj);
- }
- }
-#endif
return OK;
}
