[PATCH] Fix segfault serving mod_file_cache'ed files

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

This patch fixes a segfault I see serving files cached by MMapFile. 
 First request is okay, subsequent request  segfaults in an mmap ring 
macro during apr_brigade_destroy because the next pointer is null.. I 
did not spend time trying to figure out exactly why the next pointer was 
null, but looking at the code, it seemed quite bogus that we were 
placing an apr_mmap_t allocated out of the cmd pool into the brigade.

BTW, I am +1 on merging Cliff's mod_file_cache patch to the cleanups 
into APACHE_2_0_BRANCH. It looks correct to me.

Bill

Index: mod_file_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/cache/mod_file_cache.c,v
retrieving revision 1.73.2.1
diff -u -r1.73.2.1 mod_file_cache.c
--- mod_file_cache.c    22 Jan 2003 05:22:42 -0000    1.73.2.1
+++ mod_file_cache.c    23 Jan 2003 15:54:57 -0000
@@ -349,9 +320,11 @@
 #if APR_HAS_MMAP
     conn_rec *c = r->connection;
     apr_bucket *b;
+    apr_mmap_t *mm;
     apr_bucket_brigade *bb = apr_brigade_create(r->pool, c->bucket_alloc);
 
-    b = apr_bucket_mmap_create(file->mm, 0, (apr_size_t)file->finfo.size,
+    apr_mmap_dup(&mm, file->mm, r->pool, 0);
+    b = apr_bucket_mmap_create(mm, 0, (apr_size_t)file->finfo.size,
                                c->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(bb, b);
     b = apr_bucket_eos_create(c->bucket_alloc);

Re: [PATCH] Fix segfault serving mod_file_cache'ed files

[Cliff Woolley <jw...@virginia.edu>]

On Thu, 23 Jan 2003, Bill Stoddard wrote:

> This patch fixes a segfault I see serving files cached by MMapFile.
>  First request is okay, subsequent request  segfaults in an mmap ring
> macro during apr_brigade_destroy because the next pointer is null.. I

Ahhhhhh... right you are.  I can explain why if anybody cares.  :)  In
addition to this patch, though, you should be able to remove the
apr_mmap_dup(..., cmd->pool), as it's now redundant.  Add that, and I'm +1
for this patch, it definitely looks correct.

--Cliff