You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jw...@apache.org on 2002/04/19 08:07:50 UTC
cvs commit: apr-util/buckets apr_buckets_mmap.c
jwoolley 02/04/18 23:07:50
Modified: buckets apr_buckets_mmap.c
Log:
Fix the mmap cleanup problem seen on daedalus. This implies certain
semantics (which I think were already there anyway, but now they're
enforced):
- 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.
Revision Changes Path
1.50 +38 -3 apr-util/buckets/apr_buckets_mmap.c
Index: apr_buckets_mmap.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_buckets_mmap.c,v
retrieving revision 1.49
retrieving revision 1.50
diff -u -d -u -r1.49 -r1.50
--- apr_buckets_mmap.c 18 Apr 2002 18:34:27 -0000 1.49
+++ apr_buckets_mmap.c 19 Apr 2002 06:07:50 -0000 1.50
@@ -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;
}
Re: cvs commit: apr-util/buckets apr_buckets_mmap.c
Posted by Cliff Woolley <jw...@virginia.edu>.
On Fri, 19 Apr 2002, Greg Ames wrote:
> > jwoolley 02/04/18 23:07:50
> >
> > Modified: buckets apr_buckets_mmap.c
> > Log:
> > Fix the mmap cleanup problem seen on daedalus.
>
> :-) :-)
> I missed most of the discussion on this due to e-mail problems. Dang,
> I'm sure glad I didn't have to debug this one.
Yeah, be very glad. Anyway, did you ever come up with any definitive
cases that would reproduce the problem? I never did. I just know what
happened in a theoretical sense. :) In any case, it would be nice to see
this change [actually all of HEAD if possible] running on daedalus
sometime soon so that we can ready a release in the near term...
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: cvs commit: apr-util/buckets apr_buckets_mmap.c
Posted by Greg Ames <gr...@nc.rr.com>.
jwoolley@apache.org wrote:
>
> jwoolley 02/04/18 23:07:50
>
> Modified: buckets apr_buckets_mmap.c
> Log:
> Fix the mmap cleanup problem seen on daedalus.
:-) :-)
I missed most of the discussion on this due to e-mail problems. Dang, I'm sure
glad I didn't have to debug this one.
Greg