apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@rkbloom.net>
Subject Re: [PATCH] Strawman at fixing disjoint process locking
Date Fri, 04 Jun 2004 17:11:16 GMT

On Fri, 4 Jun 2004, Justin Erenkrantz wrote:

> --On Friday, June 4, 2004 9:58 AM -0400 rbb@rkbloom.net wrote:
> > You don't need apr_*_mutex_join, that is what apr_*_mutex_child_init is
> > supposed to do, but it can't, because the API doesn't work.  The simple
> > fact that you needed to insert a new method to do exactly what an existing
> > method does proves that the API is currently broken.
> I think we disagree on what child_init should do then.  I think child_init
> should work when there is a shared address space.  And, a proposed _join could
> work when the address space is disjoint, but there is still a parent-child
> relationship.  I don't see why their functionality must be conflated.

Because there is no reason to separate them.  The two concepts are so
inherently similar, that separating them just doesn't make sense.  What
controls if you need the features of child_init or join?  It isn't the
code that uses the lock, it is the lock mechanism.  If you are using an
anonymous lock mechanism, then you need the features in child_init, but if
you are using a named lock mechanism, then you really want to use the
features in join.  It happens that child_init will work for named lock
mechanisms as long as you used fork to create the second process, but that
isn't due to design, it is a lucky hapenstance.  What really dictates
which you should use is the lock type.  So, just change child_init to take
the same parameters that you were going to pass to join, and do the right
thing based on lock type.

>  And,
> certainly, _create shouldn't be called twice (which is what testglobalmutex
> was doing) - that would never work correctly as there needs to be only one
> owner to the lock.

Everybody agrees that the test was bogus.  Will pointed it out when I
committed the test, and I agreed with him then and now.  But, don't say it
would never work correctly, because that is actually the _only_ way the
current code _can_ work when using apr_proc_create.  Is it clean?  No way,
and it doesn't make any sense at all, but it does work.  It just happens
to work by chance instead of design.

> > Now, the reason that this patch doesn't actually fix the problem is that
> > you haven't read my post about how to solve this problem.  Some mutex
> > types can't be shared without using fork.  Unless you take that into
> > account, your fix isn't correct.
> Yes, I read your earlier posts but they made no sense to me.  Even with my
> strawman patch, all of them would still require the use of fork for now.  If
> we wanted to relax that distinction, then we'd have to stop using anonymous
> shared memory segments (like via IPC_PRIVATE).  Yet, even if we did that, I
> think the API itself (with the perhaps addition of _join) would be sufficient.
> The fact we use tactics like IPC_PRIVATE are an implementation detail.  (I'm
> not sure it's worth adding this functionality though.)

OK, this is where the problem is.  Let me be abundantly clear.  Any API
that requires the use of fork ISN'T portable.  It can't be, because fork
doesn't exist on all of our platforms.  This is why the current test code
uses apr_proc_create, because that is how the code must work if it is to
be portable.  The only reason Apache hasn't hit this bug so far, is that
we have a single child process in the Windows MPM, and thus no lock is
needed.  Try using the apr_lock types on Windows though, and they just
don't work without re-creating the lock.  That is completely bogus.  Any
API that is put forward needs to work on ALL systems equally well.

Now, we do have some lock mechanisms that will _only_ work if you use
fork(), ie anonymous lock mechanisms.  The goal behind the concept I
posted was to allow people to use anonymous lock mechanisms if they were
certain they would only be on platforms that supported fork, but also
provide a way to specify that you needed a named lock because this code
was going to be used on platforms without fork, and thus it needed to
support apr_proc_create.


View raw message