apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@rkbloom.net>
Subject Re: [PATCH] Re: apr_proc_mutex is broken
Date Wed, 20 Nov 2002 00:33:25 GMT


On Tue, 19 Nov 2002, Justin Erenkrantz wrote:

> --On Tuesday, November 19, 2002 8:39 AM -0500 Jeff Trawick
> <trawick@attglobal.net> wrote:
>
> > I contemplated relatively-complex exit sequences so that children
> > didn't exit until the test was over, but life is short, and these
> > test programs should be short too.
>
> Okay, here are the problems that we identified yesterday.
> Unfortunately, we didn't have time to indicate to the list what our
> resolution is.  Hence, you guys are somewhat out of the loop and I
> think are going off in the wrong direction here.
>
> 1) testprocmutex isn't calling apr_proc_mutex_child_init
>    (note that httpd does)
> 2) SysVSem's proc mutex isn't doing anything on child_init
> 3) In the SysVSem cleanup, semctl isn't properly identifying errors
> 4) All children are running the cleanups where only the parent should
>
> What we said we should do is that the child_init call should remove
> the cleanup from the children (since it can do the cleanup_kill as
> the SysV cleanup is a static call within proc_mutex.c).  The proc
> mutex cleanups should only be done by the parent not each child.

I don't care if the code _can_ do the cleanup with a static function.  It
SHOULDN'T do the cleanup with a static function.  By using a static
function, you have taken a great deal of flexibility away from the APR
developer, because they can't remove the cleanup anymore.  That is
completely bogus.

There is absolutely no way to say that the proc mutex cleanups should only
be done by the parent, not the child, and even if there was, it would be
wrong.

There are two types of proc mutexes, file-backed and semaphore.  As I
explained in detail, file-backed mutexes need to be cleaned up in both
parent and child, while semaphores should only be cleaned in the parent.

> I believe Aaron's initial intent of his reply to Philip was that he
> couldn't reproduce it, but that he (and the rest of the Las Vegas
> gang) did know exactly what to do to fix this correctly.  -- justin

Well, you have missed the most important fix, which is to fix the test
case.  As I explained in detail earlier today, the test program is bogus.
If the test program had been correct, this bug would not have happened.

I posted the exact fix for this earlier today.  It is three fold, and it
is the only fix that has been posted that will fix this problem correctly.

1)  apr_proc_mutex_child_init needs to do the right thing for all mutex
types.  File-backed mutexes leave it alone, other mutexes get the cleanup
removed.

2)  All mutex code must use apr_*_mutex_destroy when registering pool
cleanups.  In fact, throughout the APR code, the pool cleanups must be
registered with public functions.

3)  The test case must be fixed so that it doesn't call apr_terminate
without calling apr_initialize.

If you see an error in my logic, please tell me.  But do not tell me that
a bunch of you got together off-line and decided how to fix this.
According to what you are saying above, your fix is not correct, and it
doesn't solve the problem.  See above for details.

Ryan



Mime
View raw message