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 05:49:27 GMT

On Tue, 19 Nov 2002, Aaron Bannert wrote:

> On Tuesday, November 19, 2002, at 12:34  PM, <rbb@rkbloom.net> wrote:
>
> >
> > 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);
> >
>
> This is silly, if you do not register any cleanups and let the app
> decide
> what it wants to do. The app can register a cleanup in a pool if they
> would like that pool to control the mutex's scope.

By allocating the mutex in the pool they have already stated that the pool
should control the mutex's scope.  That is the meaning of allocating a
varibale inside a pool.  This is why pools are passed to ALL functions.

BTW, you can't leave this up to the app.  The abstraction layer makes it
too complex to have the app take care of this.  I thought at first that
the app should just kill the cleanup, but that is too hard.  If you make
the app register the cleanup, you will have code like:

if (mutex->type == (fcntl || flock)) {
    register_cleanup
else
    no-op

The problem is that some lock types need to be cleaned in the child
process while others don't.

> > 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.
>
> The cleanup function is private to the mutex implementation, it *can't*
> be made public. (There are many cleanup routines -- autoconf decides
> which
> one gets used.)

Aaron, I'm sorry, but you just aren't paying attention to the code.  For
all intents and purposes the cleanup function _is_ already public.  The
name of the function (as I've been saying all day), is
apr_proc_mutex_destroy.  As proof, read the code:

apr_proc_mutex_destroy calls mutex->meth->destroy

mutex->meth->destroy is always proc_mutex_(\.*)_destroy, where .* is one
of:
	proc_pthread, fcntl, flock, sysv.

If you read ALL of those functions, they are all:

    if ((rv = proc_mutex_(.*)_cleanup(mutex)) == APR_SUCCESS) {
        apr_pool_cleanup_kill(mutex->pool, mutex, (.*)_sysv_cleanup);
        return APR_SUCCESS;
    }
    return rv;

which means that the cleanup as it is registered today is equivalent to
apr_proc_mutex_destroy.  The functions that they call are equivalent.  I
am making the change now to use a public function for the cleanup, because
anything else is completely bogus.

If you read my messages from earlier today, I have already answered all of
the questions that you posted in each of the 5 messages you just sent.
But one more time:

The parent process allocates the mutex out of whatever pool should control
the scope.

The child processesmust call apr_proc_mutex_child_init.  That function
defines the scope of the mutex for the child process.  If the mutex type
requires a cleanup in the child process, then one is registered.

As for the bug that was hit.  Programs _MUST_ have an equal number of
apr_initialize and apr_terminate calls.  If it doesn't, then bugs like
this will happen.

Ryan




Mime
View raw message