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 2003/07/02 07:25:44 UTC
cvs commit: apr-util/include apr_buckets.h
jwoolley 2003/07/01 22:25:44
Modified: buckets apr_buckets_alloc.c
include apr_buckets.h
Log:
an addition to the api to allow httpd mpm's to share an apr_allocator_t
between a thread pool and the thread's bucket allocator. this will allow
the freelist max size to be managed.
Revision Changes Path
1.10 +9 -1 apr-util/buckets/apr_buckets_alloc.c
Index: apr_buckets_alloc.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_buckets_alloc.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -d -u -r1.9 -r1.10
--- apr_buckets_alloc.c 1 Jan 2003 00:02:17 -0000 1.9
+++ apr_buckets_alloc.c 2 Jul 2003 05:25:44 -0000 1.10
@@ -90,10 +90,18 @@
APU_DECLARE_NONSTD(apr_bucket_alloc_t *) apr_bucket_alloc_create(apr_pool_t *p)
{
apr_allocator_t *allocator;
+
+ apr_allocator_create(&allocator);
+ return apr_bucket_alloc_create_ex(p, allocator);
+}
+
+APU_DECLARE_NONSTD(apr_bucket_alloc_t *) apr_bucket_alloc_create_ex(
+ apr_pool_t *p,
+ apr_allocator_t *allocator)
+{
apr_bucket_alloc_t *list;
apr_memnode_t *block;
- apr_allocator_create(&allocator);
block = apr_allocator_alloc(allocator, ALLOC_AMT);
list = (apr_bucket_alloc_t *)block->first_avail;
list->pool = p;
1.151 +11 -1 apr-util/include/apr_buckets.h
Index: apr_buckets.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_buckets.h,v
retrieving revision 1.150
retrieving revision 1.151
diff -u -d -u -r1.150 -r1.151
--- apr_buckets.h 13 Mar 2003 23:53:19 -0000 1.150
+++ apr_buckets.h 2 Jul 2003 05:25:44 -0000 1.151
@@ -921,13 +921,23 @@
/* ***** Bucket freelist functions ***** */
/**
- * Create a bucket allocator.
+ * Create a bucket allocator (and its underlying apr_allocator_t).
* @param p Pool to allocate the allocator from [note: this is only
* used to allocate internal structures of the allocator, NOT
* to allocate the memory handed out by the allocator]
* @warning The allocator must never be used by more than one thread at a time.
*/
APU_DECLARE_NONSTD(apr_bucket_alloc_t *) apr_bucket_alloc_create(apr_pool_t *p);
+
+/**
+ * Create a bucket allocator (specifying an apr_allocator_t for it to use).
+ * @param p Pool to allocate the allocator from [note: this is only
+ * used to allocate internal structures of the allocator, NOT
+ * to allocate the memory handed out by the allocator]
+ * @param allocator The apr_allocator_t from which to get blocks of memory.
+ * @warning The allocator must never be used by more than one thread at a time.
+ */
+APU_DECLARE_NONSTD(apr_bucket_alloc_t *) apr_bucket_alloc_create_ex(apr_pool_t *p, apr_allocator_t *allocator);
/**
* Destroy a bucket allocator.
Re: cvs commit: apr-util/include apr_buckets.h
Posted by Cliff Woolley <jw...@virginia.edu>.
On Tue, 1 Jul 2003, Greg Stein wrote:
> Eh? Why the flag? What is that for... doesn't the caller know when and if he
> should clean up? So there shouldn't be a need for a flag...
The caller knows. But right now apr_buckets_alloc.c:alloc_cleanup() calls
apr_allocator_destroy(allocator) regardless of whether it "owns" the
allocator or not.
But I think this is irrelevant because I think there's an entirely cleaner
way to do this. No flag, no extra API function. Namely, we should just
have apr_bucket_alloc_create(apr_pool_t *p) {} use p's allocator always
(apr_pool_allocator_get(p)). I think that will work fine and solve this
problem for good. But I have to think about the ramifications. From what
I've seen of the MPM's in looking through them just now, I think it will
be okay. But I'll need to investigate further to be sure.
--Cliff
Re: cvs commit: apr-util/include apr_buckets.h
Posted by Cliff Woolley <jw...@virginia.edu>.
On Tue, 1 Jul 2003, Greg Stein wrote:
> Eh? Why the flag? What is that for... doesn't the caller know when and if he
> should clean up? So there shouldn't be a need for a flag...
The caller knows. But right now apr_buckets_alloc.c:alloc_cleanup() calls
apr_allocator_destroy(allocator) regardless of whether it "owns" the
allocator or not.
But I think this is irrelevant because I think there's an entirely cleaner
way to do this. No flag, no extra API function. Namely, we should just
have apr_bucket_alloc_create(apr_pool_t *p) {} use p's allocator always
(apr_pool_allocator_get(p)). I think that will work fine and solve this
problem for good. But I have to think about the ramifications. From what
I've seen of the MPM's in looking through them just now, I think it will
be okay. But I'll need to investigate further to be sure.
--Cliff
Re: cvs commit: apr-util/include apr_buckets.h
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Jul 02, 2003 at 02:03:39AM -0400, Cliff Woolley wrote:
> On Wed, 2 Jul 2003 jwoolley@apache.org wrote:
>
> > jwoolley 2003/07/01 22:25:44
> >
> > Modified: buckets apr_buckets_alloc.c
> > include apr_buckets.h
> > Log:
> > an addition to the api to allow httpd mpm's to share an apr_allocator_t
> > between a thread pool and the thread's bucket allocator. this will allow
> > the freelist max size to be managed.
>
> I just realized there's a problem with this... if you call
> apr_bucket_alloc_create_ex() and pass it an allocator, alloc_cleanup()
> should not call apr_allocator_destroy(allocator)... that should be the job
> of the caller. I'm going to have to add a flag to the apr_bucket_alloc_t
> to keep track of this.
Eh? Why the flag? What is that for... doesn't the caller know when and if he
should clean up? So there shouldn't be a need for a flag...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr-util/include apr_buckets.h
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Jul 02, 2003 at 02:03:39AM -0400, Cliff Woolley wrote:
> On Wed, 2 Jul 2003 jwoolley@apache.org wrote:
>
> > jwoolley 2003/07/01 22:25:44
> >
> > Modified: buckets apr_buckets_alloc.c
> > include apr_buckets.h
> > Log:
> > an addition to the api to allow httpd mpm's to share an apr_allocator_t
> > between a thread pool and the thread's bucket allocator. this will allow
> > the freelist max size to be managed.
>
> I just realized there's a problem with this... if you call
> apr_bucket_alloc_create_ex() and pass it an allocator, alloc_cleanup()
> should not call apr_allocator_destroy(allocator)... that should be the job
> of the caller. I'm going to have to add a flag to the apr_bucket_alloc_t
> to keep track of this.
Eh? Why the flag? What is that for... doesn't the caller know when and if he
should clean up? So there shouldn't be a need for a flag...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr-util/include apr_buckets.h
Posted by Cliff Woolley <jw...@virginia.edu>.
On Wed, 2 Jul 2003 jwoolley@apache.org wrote:
> jwoolley 2003/07/01 22:25:44
>
> Modified: buckets apr_buckets_alloc.c
> include apr_buckets.h
> Log:
> an addition to the api to allow httpd mpm's to share an apr_allocator_t
> between a thread pool and the thread's bucket allocator. this will allow
> the freelist max size to be managed.
I just realized there's a problem with this... if you call
apr_bucket_alloc_create_ex() and pass it an allocator, alloc_cleanup()
should not call apr_allocator_destroy(allocator)... that should be the job
of the caller. I'm going to have to add a flag to the apr_bucket_alloc_t
to keep track of this.
At any rate, the MPM changes in httpd are turning into a lot of ugliness
the deeper I dig. Several MPM's create their MPM's in the wrong spot or
are in some other way not being cooperative. I'll commit the changes
tomorrow to httpd-2.1 anyway, but it's likely to take a week or so to make
sure I've not introduced accidentally any double-free conditions on
shutdown or memory leaks or anything else bad. I'll need the people who
keep up with the MPM's (*all* of them) to test the changes after they're
made in 2.1-dev.
Bottom line: httpd 2.0.47 should not be held up for this.
--Cliff
Re: cvs commit: apr-util/include apr_buckets.h
Posted by Cliff Woolley <jw...@virginia.edu>.
On Wed, 2 Jul 2003 jwoolley@apache.org wrote:
> jwoolley 2003/07/01 22:25:44
>
> Modified: buckets apr_buckets_alloc.c
> include apr_buckets.h
> Log:
> an addition to the api to allow httpd mpm's to share an apr_allocator_t
> between a thread pool and the thread's bucket allocator. this will allow
> the freelist max size to be managed.
I just realized there's a problem with this... if you call
apr_bucket_alloc_create_ex() and pass it an allocator, alloc_cleanup()
should not call apr_allocator_destroy(allocator)... that should be the job
of the caller. I'm going to have to add a flag to the apr_bucket_alloc_t
to keep track of this.
At any rate, the MPM changes in httpd are turning into a lot of ugliness
the deeper I dig. Several MPM's create their MPM's in the wrong spot or
are in some other way not being cooperative. I'll commit the changes
tomorrow to httpd-2.1 anyway, but it's likely to take a week or so to make
sure I've not introduced accidentally any double-free conditions on
shutdown or memory leaks or anything else bad. I'll need the people who
keep up with the MPM's (*all* of them) to test the changes after they're
made in 2.1-dev.
Bottom line: httpd 2.0.47 should not be held up for this.
--Cliff