Re: (detabifyied) Re: [PATCH] Page Fault in mod_mem_cache-steaming response

[Bill Stoddard <bi...@wstoddard.com>]

Jean-Jacques Clar wrote:

> Just replaced tabs with spaces to follow guidelines.
> 
>  >>> JJCLAR@novell.com 12/11/2003 5:00:31 PM >>>
> Bugzilla Defect #21285
> This is a rework of the already posted patch.
>  
> It address the following situation;
> 1- request comes in for streaming response
> 2- before that request could be completed, the entry is ejected from the 
> cache
> 3- when completing the write body step, the incomplete entry is removed
> before inserting the correct entry.
>  
> The problem is that the incomplete entry was already removed/replaced
> in the cache. The cache_remove() will then seg fault.
> I don't think it has something to do with the cache size has stated
> in the bug description. I will follow with a patch removing the deprecated
> cache_size and object_cnt from the mem_cache_conf struct.
>  
> @@ -1028,7 +1029,18 @@
>                  if (sconf->lock) {
>                      apr_thread_mutex_lock(sconf->lock);
>                  }
> -                cache_remove(sconf->cache_cache, obj);
> +                /* We need to check if the object has been 
> removed/replaced
> +                 * from/in the cache, this could happen in some cases, 
> because
> +                 * the current request is a streaming response that is 
> handled
> +                 * in multiple individual pieces. - fixing Bugzilla 
> Defect 21285
> +                 */
> +                if((tmp_obj = (cache_object_t *) 
> cache_find(sconf->cache_cache, obj->key)) &&
> +                    (tmp_obj == obj)) {
> +                    cache_remove(sconf->cache_cache, obj);
> +                }
> +                else {
> +                    obj->cleanup = 0;
> +                }
> I added the patch to the bug list of attachment.
>  
> Thank you,
>  
> Jean-Jacques

I've not had time to test this but I believe it is a bit cleaner and altogether avoids the issue of duplicate 
entries in the cache. What do you think?

Index: mod_mem_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
retrieving revision 1.99
diff -u -r1.99 mod_mem_cache.c
--- mod_mem_cache.c	10 Dec 2003 03:22:32 -0000	1.99
+++ mod_mem_cache.c	16 Dec 2003 00:02:56 -0000
@@ -1043,7 +1043,25 @@
                  if (sconf->lock) {
                      apr_thread_mutex_lock(sconf->lock);
                  }
-                cache_remove(sconf->cache_cache, obj);
+                if (obj->cleanup) {
+                    /* If obj->cleanup is set, the object has been prematurly
+                     * ejected from the cache by the garbage collector. Add the
+                     * object back to the cache. If an object with the same key is
+                     * found in the cache, eject it in favor of the completed obj.
+                     */
+                    cache_object_t *tmp_obj =
+                      (cache_object_t *) cache_find(sconf->cache_cache, obj->key);
+                    if (tmp_obj) {
+                        cache_remove(sconf->cache_cache, tmp_obj);
+                        sconf->object_cnt--;
+                        sconf->cache_size -= mobj->m_len;
+                        tmp_obj->cleanup = 1;
+                    }
+                    obj->cleanup = 0;
+                }
+                else {
+                    cache_remove(sconf->cache_cache, obj);
+                }
                  mobj->m_len = obj->count;
                  cache_insert(sconf->cache_cache, obj);
                  sconf->cache_size -= (mobj->m_len - obj->count);