You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Schaefer <jo...@sunstarsys.com> on 2005/05/13 05:10:59 UTC
[PATCH] worker.c: make pconf the parent for ptrans
Without this patch, the new apr_pool_join stuff in apr's trunk
segfaults all over the place.
Index: server/mpm/worker/worker.c
===================================================================
--- server/mpm/worker/worker.c (revision 168432)
+++ server/mpm/worker/worker.c (working copy)
@@ -710,7 +710,7 @@
apr_allocator_create(&allocator);
apr_allocator_max_free_set(allocator, ap_max_mem_free);
- apr_pool_create_ex(&ptrans, NULL, NULL, allocator);
+ apr_pool_create_ex(&ptrans, pconf, NULL, allocator);
apr_allocator_owner_set(allocator, ptrans);
}
else {
--
Joe Schaefer
Re: [PATCH] worker.c: make pconf the parent for ptrans
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Jeff Trawick <tr...@gmail.com> writes:
> On 5/12/05, Joe Schaefer <jo...@sunstarsys.com> wrote:
>> Without this patch, the new apr_pool_join stuff in apr's trunk
>> segfaults all over the place.
>
> Why? Is worker MPM doing something wrong (creating standalone pool in
> that manner), or does APR need some help?
I don't know what the core problem is, but I've compiled everything
with --enable-pool-debug. There are lots of places in the code where
tables created with the pconf pool are overlaid with a table created
with the ptrans pool.
--
Joe Schaefer
Re: [PATCH] worker.c: make pconf the parent for ptrans
Posted by Joe Orton <jo...@redhat.com>.
On Mon, May 16, 2005 at 04:49:10PM -0400, Joe Schaefer wrote:
> Jeff Trawick <tr...@gmail.com> writes:
>
> > On 5/12/05, Joe Schaefer <jo...@sunstarsys.com> wrote:
> >> Without this patch, the new apr_pool_join stuff in apr's trunk
> >> segfaults all over the place.
> >
> > Why? Is worker MPM doing something wrong (creating standalone pool in
> > that manner), or does APR need some help?
(In case it wasn't clear this is triggering the new pool-lifetime
abort()s in the table code... and I can reproduce it now that I worked
out why my worker/pool-debug build didn't actually have pool-debug
enabled!)
> >> Index: server/mpm/worker/worker.c
> >
> >> apr_allocator_create(&allocator);
> >> apr_allocator_max_free_set(allocator, ap_max_mem_free);
> >> - apr_pool_create_ex(&ptrans, NULL, NULL, allocator);
> >> + apr_pool_create_ex(&ptrans, pconf, NULL, allocator);
I think this is logically correct:
- the ptrans pools are destroyed when the queue_info object is cleaned up
- the queue_info object is cleaned up when pchild is destroyed
- hence, ptrans must have a lifetime at least as long as pchild
since pconf is the parent of pchild it satisfies the constraint.
> > Isn't this on the listener thread? Do we need to be mucking with the
> > pconf pool on non-main thread? Sounds dangerous (non-thread-safe).
>
> Yes, it's on a listener thread. Looking at apr_pool_create_ex,
> it does take a mutex on pconf, so I think it's a thread-safe operation.
> All this does is use the pconf pool instead of the global pool:
and subpool creation is thread-safe and must be either way since a NULL
parent == global pool.
I've committed the change - thanks.
joe
Re: [PATCH] worker.c: make pconf the parent for ptrans
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Jeff Trawick <tr...@gmail.com> writes:
> On 5/12/05, Joe Schaefer <jo...@sunstarsys.com> wrote:
>> Without this patch, the new apr_pool_join stuff in apr's trunk
>> segfaults all over the place.
>
> Why? Is worker MPM doing something wrong (creating standalone pool in
> that manner), or does APR need some help?
>
>> Index: server/mpm/worker/worker.c
>
>> apr_allocator_create(&allocator);
>> apr_allocator_max_free_set(allocator, ap_max_mem_free);
>> - apr_pool_create_ex(&ptrans, NULL, NULL, allocator);
>> + apr_pool_create_ex(&ptrans, pconf, NULL, allocator);
>
> Isn't this on the listener thread? Do we need to be mucking with the
> pconf pool on non-main thread? Sounds dangerous (non-thread-safe).
Yes, it's on a listener thread. Looking at apr_pool_create_ex,
it does take a mutex on pconf, so I think it's a thread-safe operation.
All this does is use the pconf pool instead of the global pool:
APR_DECLARE(apr_status_t) apr_pool_create_ex(apr_pool_t **newpool,
apr_pool_t *parent,
apr_abortfunc_t abort_fn,
apr_allocator_t *allocator)
{
apr_pool_t *pool;
apr_memnode_t *node;
*newpool = NULL;
if (!parent)
parent = global_pool;
--
Joe Schaefer
Re: [PATCH] worker.c: make pconf the parent for ptrans
Posted by Jeff Trawick <tr...@gmail.com>.
On 5/12/05, Joe Schaefer <jo...@sunstarsys.com> wrote:
> Without this patch, the new apr_pool_join stuff in apr's trunk
> segfaults all over the place.
Why? Is worker MPM doing something wrong (creating standalone pool in
that manner), or does APR need some help?
> Index: server/mpm/worker/worker.c
> apr_allocator_create(&allocator);
> apr_allocator_max_free_set(allocator, ap_max_mem_free);
> - apr_pool_create_ex(&ptrans, NULL, NULL, allocator);
> + apr_pool_create_ex(&ptrans, pconf, NULL, allocator);
Isn't this on the listener thread? Do we need to be mucking with the
pconf pool on non-main thread? Sounds dangerous (non-thread-safe).