Attached is a patch for the mmap bucket cleanup issue. The caveats are
that it only works if:
- if the apr_mmap_t passed to apr_bucket_mmap_create/make is the owner of
the mmaped region, the mmap buckets code assumes complete
responsibility for it. In particular, external code should NEVER call
apr_mmap_delete or apr_mmap_dup (transferring ownership) on that
apr_mmap_t after it's been placed in the bucket.
- if the apr_mmap_t passed to apr_bucket_mmap_create/make is NOT the
owner of the mmaped region, then the creator of the bucket is
absolutely guaranteeing us that the apr_mmap_t will live longer than
the bucket.
I think this matches the existing semantics, so it shouldn't be that big a
deal. HOWEVER, note that this patch causes two of the apache/limits.t
tests (7 and 9) to fail with an assertion pop, though I think this
represents a legitimate bug in Apache. Basically, the buckets code now
enforces the fact that you absolutely positively CANNOT access an mmap
bucket except to delete it after the apr_mmap_t in it gets cleaned up.
--Cliff
This backtrace is from test 7 of limits.t:
Program received signal SIGABRT, Aborted.
0x403139d1 in kill () from /lib/libc.so.6
(gdb) bt
#0 0x403139d1 in kill () from /lib/libc.so.6
#1 0x402d9c9e in pthread_kill () from /lib/libpthread.so.0
#2 0x402da16d in raise () from /lib/libpthread.so.0
#3 0x40314e31 in abort () from /lib/libc.so.6
#4 0x806a329 in ap_log_assert (szExp=0x80a34db "readbytes > 0",
szFile=0x80a33a4 "http_protocol.c", nLine=886) at log.c:642
#5 0x805f157 in ap_http_filter (f=0x819e4b8, b=0x819eaa8,
mode=AP_MODE_GETLINE, block=APR_BLOCK_READ, readbytes=0)
at http_protocol.c:886
#6 0x8073142 in ap_get_brigade (next=0x819e4b8, bb=0x819eaa8,
mode=AP_MODE_GETLINE, block=APR_BLOCK_READ, readbytes=0)
at util_filter.c:507
#7 0x807c58a in net_time_filter (f=0x819dd50, b=0x819eaa8,
mode=AP_MODE_GETLINE, block=APR_BLOCK_READ, readbytes=0) at
core.c:3278
#8 0x8073142 in ap_get_brigade (next=0x819dd50, bb=0x819eaa8,
mode=AP_MODE_GETLINE, block=APR_BLOCK_READ, readbytes=0)
at util_filter.c:507
#9 0x807450b in ap_rgetline_core (s=0xbfffcd98, n=30, read=0xbfffcd90,
r=0x819d5b0, fold=0) at protocol.c:222
#10 0x8074b24 in ap_getline (s=0xbfffcdc8 "\016", n=30, r=0x819d5b0,
fold=0)
at protocol.c:538
#11 0x805ef2c in ap_http_filter (f=0x819e4b8, b=0x819dd30,
mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
at http_protocol.c:835
#12 0x8073142 in ap_get_brigade (next=0x819e4b8, bb=0x819dd30,
mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
at util_filter.c:507
#13 0x807c58a in net_time_filter (f=0x819dd50, b=0x819dd30,
mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
at core.c:3278
#14 0x8073142 in ap_get_brigade (next=0x819dd50, bb=0x819dd30,
mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192)
at util_filter.c:507
#15 0x8060640 in ap_get_client_block (r=0x819d5b0, buffer=0xbfffcf78
"\004",
bufsiz=8192) at http_protocol.c:1663
#16 0x80608d8 in ap_discard_request_body (r=0x819d5b0) at
http_protocol.c:1757
#17 0x807c074 in default_handler (r=0x819d5b0) at core.c:3166
#18 0x8065f81 in ap_run_handler (r=0x819d5b0) at config.c:193
#19 0x80666be in ap_invoke_handler (r=0x819d5b0) at config.c:373
#20 0x8062e18 in ap_process_request (r=0x819d5b0) at http_request.c:261
#21 0x805d77e in ap_process_http_connection (c=0x8193658) at
http_core.c:291
#22 0x8070bc1 in ap_run_process_connection (c=0x8193658) at
connection.c:85
#23 0x8070fb0 in ap_process_connection (c=0x8193658, csd=0x8193588)
at connection.c:207
#24 0x8064737 in child_main (child_num_arg=0) at prefork.c:671
#25 0x806481d in make_child (s=0x80bfff0, slot=0) at prefork.c:711
#26 0x8064954 in startup_children (number_to_start=2) at prefork.c:783
#27 0x8064d86 in ap_mpm_run (_pconf=0x80be2a0, plog=0x80e8348,
s=0x80bfff0)
at prefork.c:999
#28 0x806b6c3 in main (argc=7, argv=0xbffff2e4) at main.c:632
--------------------------------------------------------------
Cliff Woolley
[EMAIL PROTECTED]
Charlottesville, VA
Index: apr_buckets_mmap.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_buckets_mmap.c,v
retrieving revision 1.49
diff -u -d -r1.49 apr_buckets_mmap.c
--- apr_buckets_mmap.c 18 Apr 2002 18:34:27 -0000 1.49
+++ apr_buckets_mmap.c 18 Apr 2002 20:29:53 -0000
@@ -63,6 +63,11 @@
apr_status_t ok;
void *addr;
+ if (!m->mmap) {
+ /* due to cleanup issues we have no choice but to check this */
+ return APR_EINVAL;
+ }
+
ok = apr_mmap_offset(&addr, m->mmap, b->start);
if (ok != APR_SUCCESS) {
return ok;
@@ -72,14 +77,27 @@
return APR_SUCCESS;
}
+static apr_status_t mmap_bucket_cleanup(void *data)
+{
+ /* the mmap has disappeared out from under us. we have no choice
+ * but to invalidate this bucket. from now on, all you can do to this
+ * bucket is destroy it, not read from it.
+ */
+ apr_bucket_mmap *m = data;
+
+ m->mmap = NULL;
+ return APR_SUCCESS;
+}
+
static void mmap_bucket_destroy(void *data)
{
apr_bucket_mmap *m = data;
if (apr_bucket_shared_destroy(m)) {
- /* if we are the owner of the mmaped region, apr_mmap_delete will
- * munmap it for us. if we're not, it's essentially a noop. */
- apr_mmap_delete(m->mmap);
+ if (m->mmap && m->mmap->is_owner) {
+ apr_pool_cleanup_kill(m->mmap->cntxt, m, mmap_bucket_cleanup);
+ apr_mmap_delete(m->mmap);
+ }
apr_bucket_free(m);
}
}
@@ -96,6 +114,11 @@
m = apr_bucket_alloc(sizeof(*m), b->list);
m->mmap = mm;
+ if (mm->is_owner) {
+ apr_pool_cleanup_register(mm->cntxt, m, mmap_bucket_cleanup,
+ apr_pool_cleanup_null);
+ }
+
b = apr_bucket_shared_make(b, m, start, length);
b->type = &apr_bucket_type_mmap;
@@ -123,6 +146,11 @@
apr_mmap_t *new_mm;
apr_status_t ok;
+ if (!mm) {
+ /* due to cleanup issues we have no choice but to check this */
+ return APR_EINVAL;
+ }
+
if (apr_pool_is_ancestor(mm->cntxt, p)) {
return APR_SUCCESS;
}
@@ -131,7 +159,14 @@
if (ok != APR_SUCCESS) {
return ok;
}
+
m->mmap = new_mm;
+ if (new_mm->is_owner) {
+ apr_pool_cleanup_kill(mm->cntxt, m, mmap_bucket_cleanup);
+ apr_pool_cleanup_register(new_mm->cntxt, m, mmap_bucket_cleanup,
+ apr_pool_cleanup_null);
+ }
+
return APR_SUCCESS;
}
