You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by rb...@covalent.net on 2001/01/06 01:52:54 UTC
Re: APR Bug or misunderstanding ?
(Copied to dev@apr.apache.org, which is the APR development mailing list.)
> I've been having a look at the new APR library on NT. To cut a long story short
> I ended up with a program that did: apr_initialize() followed immediately by
> apr_terminate(). The program blew up in the terminate call.
> After some checking I found that the poblem appeared the following:
> apr_term_alloc() destroyes the alloc_mutex and then issues
> apr_destroy_pool(globalp);
> Now inside apr_destroy_pool() it attempted to acquire the alloc_mutex lock.
> This had already been destroyed, hence the system (rightly) complained.
> I inverted the order of the operations in apr_term_alloc() so that the pool is
> destroyed before the mutexes are destroyed.
#1. Thank you for diagnosing this problem. I have not seen it, but
having reviewed the code, you are 100% correct. Your solution is slightly
flawed, but I am about to commit a different fix.
The problem with your solution is very odd, and is unlikely to happen, but
we can't just change the order. Basically, the alloc_mutex and the
spawn_mutex are allocated out of the globalp pool, so when we destroy
those pools, the mutexes go away too. This leaves a time, when we could
be destroying the globalp pool, without mutexes, even though we have
multiple pools. I am surprised your fix worked, because I would have
thought the destroy_lock calls would have seg faulted, since those mutexes
would have been freed when the pool was destroyed.
The fix I will commit instead, will check to ensure that there are locks
when we destroy a pool. This will add two if's to the destroy_pool call,
but I believe this is the safer option.
Again, thank you for the thorough diagnosis, it makes this very easy to
fix. Please let us know if you have any other problems with APR.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: APR - part 2
Posted by Gregory Nicholls <gn...@level8.com>.
I apologise, I didn't realise there was an APR specific list ...
G.
rbb@covalent.net wrote:
> Feel free to supply a patch. I would ask that from now on questions about
> APR should be sent to dev@apr.apache.org, which is the APR development
> list. Many of the Apache developers also work on APR, but there are many
> other APR developers who are not on new-httpd@apache.org.
>
> Ryan
>
> On Sat, 6 Jan 2001, Gregory Nicholls wrote:
>
> > I can do this if you like. Be warned that it will impact just about every source module. If
> > you want to leave it until a more opportune time it's not going to hurt me at all, I was just
> > curious.
> > Greg.
> >
> > rbb@covalent.net wrote:
> >
> > > > Hiya,
> > > > I've a second APR related question. Some of the external prototypes use
> > > > APR_DECLARE(foo_t) and others do not. Notably apr_create_pool() has no such declaration
> > > > wheras apr_destroy_pool() does. Is this an oversight or is there a reason for the mix ??
> > >
> > > It is a major oversight. The problem is that Unix doesn't tend to need
> > > those things, so when APR was originally written, they were forgotten. As
> > > we find them, we try to fix them, but we have had bigger issues on our
> > > plate recently.
> > >
> > > Ryan
> > >
> > > _______________________________________________________________________________
> > > Ryan Bloom rbb@apache.org
> > > 406 29th St.
> > > San Francisco, CA 94131
> > > -------------------------------------------------------------------------------
> >
> >
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
Re: APR - part 2
Posted by rb...@covalent.net.
Feel free to supply a patch. I would ask that from now on questions about
APR should be sent to dev@apr.apache.org, which is the APR development
list. Many of the Apache developers also work on APR, but there are many
other APR developers who are not on new-httpd@apache.org.
Ryan
On Sat, 6 Jan 2001, Gregory Nicholls wrote:
> I can do this if you like. Be warned that it will impact just about every source module. If
> you want to leave it until a more opportune time it's not going to hurt me at all, I was just
> curious.
> Greg.
>
> rbb@covalent.net wrote:
>
> > > Hiya,
> > > I've a second APR related question. Some of the external prototypes use
> > > APR_DECLARE(foo_t) and others do not. Notably apr_create_pool() has no such declaration
> > > wheras apr_destroy_pool() does. Is this an oversight or is there a reason for the mix ??
> >
> > It is a major oversight. The problem is that Unix doesn't tend to need
> > those things, so when APR was originally written, they were forgotten. As
> > we find them, we try to fix them, but we have had bigger issues on our
> > plate recently.
> >
> > Ryan
> >
> > _______________________________________________________________________________
> > Ryan Bloom rbb@apache.org
> > 406 29th St.
> > San Francisco, CA 94131
> > -------------------------------------------------------------------------------
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: APR - part 2
Posted by Gregory Nicholls <gn...@level8.com>.
I can do this if you like. Be warned that it will impact just about every source module. If
you want to leave it until a more opportune time it's not going to hurt me at all, I was just
curious.
Greg.
rbb@covalent.net wrote:
> > Hiya,
> > I've a second APR related question. Some of the external prototypes use
> > APR_DECLARE(foo_t) and others do not. Notably apr_create_pool() has no such declaration
> > wheras apr_destroy_pool() does. Is this an oversight or is there a reason for the mix ??
>
> It is a major oversight. The problem is that Unix doesn't tend to need
> those things, so when APR was originally written, they were forgotten. As
> we find them, we try to fix them, but we have had bigger issues on our
> plate recently.
>
> Ryan
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
Re: APR - part 2
Posted by rb...@covalent.net.
> Hiya,
> I've a second APR related question. Some of the external prototypes use
> APR_DECLARE(foo_t) and others do not. Notably apr_create_pool() has no such declaration
> wheras apr_destroy_pool() does. Is this an oversight or is there a reason for the mix ??
It is a major oversight. The problem is that Unix doesn't tend to need
those things, so when APR was originally written, they were forgotten. As
we find them, we try to fix them, but we have had bigger issues on our
plate recently.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
APR - part 2
Posted by Gregory Nicholls <gn...@level8.com>.
Hiya,
I've a second APR related question. Some of the external prototypes use
APR_DECLARE(foo_t) and others do not. Notably apr_create_pool() has no such declaration
wheras apr_destroy_pool() does. Is this an oversight or is there a reason for the mix ??
Thanks,
Gregory Nicholls.
RE: APR Bug or misunderstanding ?
Posted by rb...@covalent.net.
> > Hmmmm..... I have just looked at the code in more detail, we have already
> > an "if (alloc_mutex {" check both before locking and unlocking the
> > mutex. This means that we shouldn't be seg faulting. When did you grab
> > your copy of APR? Do you have the if calls in your copy?
>
> Part of the problem I think is that apr_destroy_lock does
> not null out the alloc_mutex pointer so that the if(alloc_mutex)
> test in apr_destroy_pool is true and we trap (at least on Windows).
> Passing in a apr_lock_t** intro apr_destroy_lock and nulling the pointer
> might fix the problem.
Actually, I think that is the whole problem. We can't really pass a
pointer to the lock to lock_destroy, because that would make the cleanup a
bit uglier, and the function wouldn't look like the rest of the APR
functions. However, we can very easily set the sock to NULL before
destroying the pglobal lock. I'll commit that now.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
RE: APR Bug or misunderstanding ?
Posted by Allan Edwards <ak...@meepzor.com>.
> Hmmmm..... I have just looked at the code in more detail, we have already
> an "if (alloc_mutex {" check both before locking and unlocking the
> mutex. This means that we shouldn't be seg faulting. When did you grab
> your copy of APR? Do you have the if calls in your copy?
Part of the problem I think is that apr_destroy_lock does
not null out the alloc_mutex pointer so that the if(alloc_mutex)
test in apr_destroy_pool is true and we trap (at least on Windows).
Passing in a apr_lock_t** intro apr_destroy_lock and nulling the pointer
might fix the problem.
I had noticed this problem in Apache this afternoon but hadn't
had time to investigate. BTW I am having chronic mail delivery problems
so if this has already been answered please ignore.
Allan
Re: APR Bug or misunderstanding ?
Posted by rb...@covalent.net.
On Fri, 5 Jan 2001 rbb@covalent.net wrote:
>
> (Copied to dev@apr.apache.org, which is the APR development mailing list.)
>
> > I've been having a look at the new APR library on NT. To cut a long story short
> > I ended up with a program that did: apr_initialize() followed immediately by
> > apr_terminate(). The program blew up in the terminate call.
> > After some checking I found that the poblem appeared the following:
> > apr_term_alloc() destroyes the alloc_mutex and then issues
> > apr_destroy_pool(globalp);
> > Now inside apr_destroy_pool() it attempted to acquire the alloc_mutex lock.
> > This had already been destroyed, hence the system (rightly) complained.
> > I inverted the order of the operations in apr_term_alloc() so that the pool is
> > destroyed before the mutexes are destroyed.
>
> #1. Thank you for diagnosing this problem. I have not seen it, but
> having reviewed the code, you are 100% correct. Your solution is slightly
> flawed, but I am about to commit a different fix.
>
> The fix I will commit instead, will check to ensure that there are locks
> when we destroy a pool. This will add two if's to the destroy_pool call,
> but I believe this is the safer option.
Hmmmm..... I have just looked at the code in more detail, we have already
an "if (alloc_mutex {" check both before locking and unlocking the
mutex. This means that we shouldn't be seg faulting. When did you grab
your copy of APR? Do you have the if calls in your copy?
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------