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 Tue, 19 Nov 2002 20:34:05 GMT

On 19 Nov 2002, Philip Martin wrote:

> <rbb@rkbloom.net> writes:
> > If you don't want the child process to destroy the mutex, then you should
> > kill that cleanup in the child process.  That is why we have the
> > apr_pool_cleanup_kill API.
> Are you suggesting the application should do this, or the library?

The app would have to do this.  The easiest way to do this, is to change
APR so that all cleanups are registered as apr_proc_mutex_destroy.  Then,
the app just needs to call

    apr_pool_cleanup_kill(pool, mutex, apr_proc_mutex_destroy);

No new API required.  In fact, I actually consider it a VERY big bug that
apps can't currently kill the cleanup.  To the best of my knowledge, this
is the only APR sub-system where the APR developer doesn't have that kind
of control.

> Having the library do it doesn't sound very robust, when would you do
> it?  At apr_proc_fork time?  What about a third party library that
> doesn't use APR, calls fork directly and later runs atexit handlers?

Yes, I agree doing it in the library is incredibly non-robust.

> The application cannot do it at present because it has no access to
> the cleanup handler.  So this would mean adding a function to the
> proc_mutex interface, apr_proc_mutex_cleanup_kill perhaps?  We would
> need apr_global_mutex_cleanup_kill as well.  Even then, an application
> using a proc_mutex has to know about every use of fork/apr_proc_fork
> in every library.

See above, no new functions are needed, APR just needs to be modified to
use the external APIs that are already provided when it registers a
cleanup.  As for the app needing to know about every use of
fork/apr_proc_fork in every library, that isn't true.  The app needs to
know about every use of apr_terminate in every library.  It is the
apr_terminate call that is causing the problem, not the actual fork()
call.  However, if the app removes the proc_mutex cleanup before the new
process is spawned, it won't have to worry about this, because the child
process won't have a cleanup to kill.

The only case that is really hard to deal with here, is when the parent
process wants to keep the cleanup and the child process wants to kill it.
This is why we created the inherit APIs a while ago, and the locking
system should be migrated to use it.

> I think the current pool cleanup handler is a mistake.  The handler
> should cleanup only those resources local to the process, leaving the
> proc_mutex in a working state.  It should be the application's
> responsibility to call (or fail to call :) apr_proc_mutex_destroy at
> the appropriate place.

The hard part is figuring out which resource is local to the process.
Some locks need to be deleted in each child, some can't be.  The other
option to solve this, is to actually use the apr_proc_mutex_child_init
function correctly.  If the mutex type can't be deleted in the child, then
apr_proc_mutex_child_init should remove that cleanup.

Regardless, the lock code needs to use the correct cleanup function, which
is the public function, so that the APR developer _can_ remove the cleanup
if they want to.


View raw message