apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron Bannert <aa...@clove.org>
Subject Re: [PATCH] Re: apr_proc_mutex is broken
Date Wed, 20 Nov 2002 05:07:46 GMT

On Tuesday, November 19, 2002, at 01:51  PM, <rbb@rkbloom.net> wrote:
>
> 	apr_proc_mutex_child_init()
> 		Depending on mutex type, the cleanup should be killed or
> not.  In general, file-based mutex will be killed, but semaphores will
> not.
>
I don't understand this, why does the app treat the different types
differently, or are you saying that the cleanup killing does or
doesn't happen depending on the implementation?

> 2)  We aren't calling apr_proc_mutex_child_init, which is what makes 
> the
> mutex available to the child process.  We were relying on inheritance 
> to
> get the mutex to the child process.  If we had been calling this 
> function
> (and the function was fixed to kill the cleanup if appropriate), then 
> we
> would have masked the previous bug.

Child init on almost all of the proc mutex implementations is a no-op 
[on
unix, where this bug exists].

> At the end of the day, the test was bogus, and it wasn't good APR code,
> which is what was causing the problem.  There is one change that 
> should be
> made to the underlying APR code to help hide this from the user, but 
> the
> real problem is in the test.

I don't think anyone disagrees that there are problems in the test, but
I do strongly believe that the interface is also broken.

> But the latter is MUCH worse when it comes to locks.  You only have so
> many semaphores available to you.  A resource leak can be a real PITA 
> to
> track down, and I suspect that this general bug is biting Apache in a
> couple of places because we do leak semaphores.  I currently have 12
> semaphores in use by Apache.  When I stop the currently running 
> server, I
> have 9 in use (all by the web server).
>
> Resource leaks suck, and they are just as hard to diagnose as doing
> something too often.

This is not a valid reason, since these kinds of errors are due to
bugs in the application, NOT due to bad APR interfaces or 
implementations.

I don't think we should complicate the heck out of APR just to hold
the hands of poor app authors.

> The real fix for this problem is to fix the test code as I have stated
> above so that it is a valid example of how to write APR code, and to 
> fix
> the library so that we register cleanups with a public function, and so
> that apr_proc_mutex_child_init does the right thing in all cases.  I 
> will
> do the work later tonight (assuming my wife doesn't go into labor 
> before
> then) unless somebody beats me to it.

I'd much rather just have APR not do any automatic cleanups on any of
the mutex types, and to leave it up to the app author to decide if they
want to register a cleanup or would rather just call the destroy()
methods themselves.

-aaron

p.s. Give Kelly my best. :)


Mime
View raw message