You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Aaron Bannert <aa...@ebuilt.com> on 2001/07/02 20:30:59 UTC
[PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
As discussed earlier on new-httpd and dev@apr, the lock scope
CROSS_PROCESS is not portable, where LOCKALL is. I've fixed
that by simply removing CROSS_PROCESS. The scopes we have left use names
that are overloaded and confusing (LOCKALL vs CROSS_PROCESS huh?)
so I went ahead and dropped the whole thing in favor of something
more consistent and disambiguous, the POSIXified names:
APR_PROCESS_PRIVATE -- (old INTRAPROCESS) synchronizes threads in the current
process.
APR_PROCESS_SHARED -- (old CROSS_PROCESS *and* LOCKALL) synchronizes threads
across processes and their threads. Essentially acts as global lock
on all platforms.
The two patches must be applied in order (apr first, then httpd) since
one depends on the other. The httpd changes were simple string
replacements, but I looked them over before submitting. The apr changes
seemed to work fine with the various apr/test/ routines on my platform
(linux 2.4), but I don't have access to all the platforms that this
patch might affect.
-aaron
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Aaron Bannert <aa...@ebuilt.com> writes:
> As discussed earlier on new-httpd and dev@apr, the lock scope
> CROSS_PROCESS is not portable, where LOCKALL is. I've fixed
> that by simply removing CROSS_PROCESS. The scopes we have left use names
> that are overloaded and confusing (LOCKALL vs CROSS_PROCESS huh?)
> so I went ahead and dropped the whole thing in favor of something
> more consistent and disambiguous, the POSIXified names:
Sorry to bring this up again, but today I was showing someone a
program of mine when I realized how dependent it is on the current
CROSS_PROCESS semantics. It won't work with LOCKALL semantics.
(This program shows me which lock mechanisms block out other threads
in the same process. I'd be lost without it :) )
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
+1. Makes the code so much easier to understand. -- justin
On Mon, Jul 02, 2001 at 11:30:59AM -0700, Aaron Bannert wrote:
> As discussed earlier on new-httpd and dev@apr, the lock scope
> CROSS_PROCESS is not portable, where LOCKALL is. I've fixed
> that by simply removing CROSS_PROCESS. The scopes we have left use names
> that are overloaded and confusing (LOCKALL vs CROSS_PROCESS huh?)
> so I went ahead and dropped the whole thing in favor of something
> more consistent and disambiguous, the POSIXified names:
>
> APR_PROCESS_PRIVATE -- (old INTRAPROCESS) synchronizes threads in the current
> process.
>
> APR_PROCESS_SHARED -- (old CROSS_PROCESS *and* LOCKALL) synchronizes threads
> across processes and their threads. Essentially acts as global lock
> on all platforms.
>
>
> The two patches must be applied in order (apr first, then httpd) since
> one depends on the other. The httpd changes were simple string
> replacements, but I looked them over before submitting. The apr changes
> seemed to work fine with the various apr/test/ routines on my platform
> (linux 2.4), but I don't have access to all the platforms that this
> patch might affect.
>
> -aaron
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Aaron Bannert <aa...@ebuilt.com> writes:
> As discussed earlier on new-httpd and dev@apr, the lock scope
> CROSS_PROCESS is not portable, where LOCKALL is. I've fixed
> that by simply removing CROSS_PROCESS. The scopes we have left use names
> that are overloaded and confusing (LOCKALL vs CROSS_PROCESS huh?)
> so I went ahead and dropped the whole thing in favor of something
> more consistent and disambiguous, the POSIXified names:
Sorry to bring this up again, but today I was showing someone a
program of mine when I realized how dependent it is on the current
CROSS_PROCESS semantics. It won't work with LOCKALL semantics.
(This program shows me which lock mechanisms block out other threads
in the same process. I'd be lost without it :) )
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 03, 2001 at 03:02:47PM -0400, Jeff Trawick wrote:
> that makes sense to me too for my own purposes, but
>
> 1) before long I suspect some folks will want/need a single shared
> library version of APR which works with multiple APR apps
>
> and it won't just be "want" because they think it will be nicer to
> have one build of APR... it will be "need" because different
> dynamically-loaded code will have to use the same copy of APR*
>
> (*maybe gstein could chime in here... some months back there was a
> discussion of why single shared library of APR was so important)
Maybe we could take the libc approach and build libapr and libapr_r.
Not sure how I like that though (my first thought - yuck!). That
sort of rubs me the wrong way.
Compiling APR with threads and using it with something that isn't
threaded won't break anything - in this case, you just may acquire an
extra potentially unnecessary lock here and there. So, don't scream
about performance. It should all work though. But, when we do *have*
threads, we need to make sure the locking is done right. That
currently isn't the case.
> 2) even if, back to the Apache example, we use the prefork MPM, we may
> have a module which uses threads; I've been told there are threaded
> modules for Apache 1.3...; presumably we'd want such modules to
> work with APR with Apache 2.0
Then, they pay the penalty that all of the stock Apache 2.0 calls to
APR now have to be locked when they don't necessarily have to be.
It's a tradeoff.
I'm not seeing any functionality loss here. Maybe I'm wrong.
-- justin
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by rb...@covalent.net.
> > I'm a bit surprised that none of the folks who were around when
> > CROSS_PROCESS vs. LOCKALL was invented have participated in the
> > discussion. I think I'm at least as concerned with that as with
> > losing a lock flavor.
>
> I am the person who introduced all of that. To be honest, I can't track
> all the e-mail anymore. I now follow what I have time to follow, and get
> involved with things I feel strongly about. For stuff I don't have strong
> opinions about, I tend to not get involved, because I just don't have the
> time. Plus, when I introduced it, we were coding on Linux only. I don't
> know that we have information of any platforms where a CROSS_PROCESS lock
> isn't LOCKALL, but I was nervous about it when I wrote the code. And, I
> was using an older version of Linux, so threads were great back then.
s/were/were not/
> It's possible this isn't really an issue. I just don't know. :-){
Ryan
_____________________________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
-----------------------------------------------------------------------------
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by rb...@covalent.net.
> that makes sense to me too for my own purposes, but
>
> 1) before long I suspect some folks will want/need a single shared
> library version of APR which works with multiple APR apps
>
> and it won't just be "want" because they think it will be nicer to
> have one build of APR... it will be "need" because different
> dynamically-loaded code will have to use the same copy of APR*
>
> (*maybe gstein could chime in here... some months back there was a
> discussion of why single shared library of APR was so important)
>
> 2) even if, back to the Apache example, we use the prefork MPM, we may
> have a module which uses threads; I've been told there are threaded
> modules for Apache 1.3...; presumably we'd want such modules to
> work with APR with Apache 2.0
>
> I'm a bit surprised that none of the folks who were around when
> CROSS_PROCESS vs. LOCKALL was invented have participated in the
> discussion. I think I'm at least as concerned with that as with
> losing a lock flavor.
I am the person who introduced all of that. To be honest, I can't track
all the e-mail anymore. I now follow what I have time to follow, and get
involved with things I feel strongly about. For stuff I don't have strong
opinions about, I tend to not get involved, because I just don't have the
time. Plus, when I introduced it, we were coding on Linux only. I don't
know that we have information of any platforms where a CROSS_PROCESS lock
isn't LOCKALL, but I was nervous about it when I wrote the code. And, I
was using an older version of Linux, so threads were great back then.
It's possible this isn't really an issue. I just don't know. :-){
Ryan
_____________________________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
-----------------------------------------------------------------------------
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@ebuilt.com> writes:
> > > This could give the non-threaded users of APR even more reason to
> > > specify "--without-threads" when configuring - which they should be
> > > doing in the first place.
> >
> > yes. And we want recompiles of APR, no?
>
> If you have a non-threaded app, you should use a non-threaded APR.
> That's what makes sense to me, at least. -- justin
that makes sense to me too for my own purposes, but
1) before long I suspect some folks will want/need a single shared
library version of APR which works with multiple APR apps
and it won't just be "want" because they think it will be nicer to
have one build of APR... it will be "need" because different
dynamically-loaded code will have to use the same copy of APR*
(*maybe gstein could chime in here... some months back there was a
discussion of why single shared library of APR was so important)
2) even if, back to the Apache example, we use the prefork MPM, we may
have a module which uses threads; I've been told there are threaded
modules for Apache 1.3...; presumably we'd want such modules to
work with APR with Apache 2.0
I'm a bit surprised that none of the folks who were around when
CROSS_PROCESS vs. LOCKALL was invented have participated in the
discussion. I think I'm at least as concerned with that as with
losing a lock flavor.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Aaron Bannert <aa...@ebuilt.com>.
On Wed, Jul 04, 2001 at 02:30:27AM -0700, Justin Erenkrantz wrote:
> > I wrote a simple APR app to test this out... I'll try to find time
> > this evening to remove some bogosity, re-test, and post the app. I
> > don't think the bogosity affects the outcome but I need to
> > double-check.
>
> But we can't run an APR app when we are in the middle of autoconf. =)
> Don't we need to have this information before we compile? I'm sensing a
> chicken and egg problem here. Please correct me if I'm wrong.
I have a feeling he didn't mean an actual APR-linked app.
Jeff, I'd love to see what you have for this when you get a chance.
> Backing up a bit to the original point of this thread. As I see it
> there were two things initially addressed in Aaron's patch:
>
> 1) Removing CROSS_PROCESS and making it equivalent to LOCK_ALL.
> On threaded APR, a CROSS_PROCESS lock may or may not implictly
> acquire a intraprocess lock. I believe this is broken. We
> shouldn't have a lock whose granularity is dependent upon the
> platform and lock mechanism. In all cases that we examined (in
> APR and httpd), the intention of the CROSS_PROCESS locks should
> really be LOCK_ALL. Because some platforms/mechanisms will acquire
> an INTRAPROCESS lock anyway with CROSS_PROCESS and others won't, the
> current behavior seems ill-defined and should be standardized.
>
> The LOCK_ALL behavior seems to be the correct one at the cost of a
> potential extra lock acquisition where we weren't expecting the
> INTRAPROCESS lock in the first place. My assumption would be
> that this requesting this behavior (interprocess but not intraprocess)
> would be very rare. I'd like to see a case where we would want to
> lock out threads from other processes and not from our own process.
> Furthermore, you can't satisfy this request anyway with the current
> CROSS_PROCESS code as whether we obtain an intraprocess lock is
> undefined. You can't assume that it'll happen.
Actually, the problem isn't that we would want to lock out threads
in other processes and not our own threads (I don't know of any mechanism,
buggy or not, that does that). The way I see it we are mixing two different
requirements into the same set of functions -- apr_lock_*()
1. eg. prefork mpm needs to synchronize access to the accept() system call
because of some lame thundering hurd problem or whatever they called it.
There would be no threading involved anywhere, just simple crossprocess
(process-group-wide really) mutexes.
2. eg. mod_acme (made that one up) allocates some shared memory to store
it's runtime state and IPC. It is running under the threaded mpm, and
therefore needs to synchronize access to this shared memory across
all involved processes and their threads.
Currently, 1) would use CROSS_PROCESS, and 2) would use LOCK_ALL. This is
fine as long as a threaded app never tries to use CROSS_PROCESS and. The
problem is that we don't enforce that, nor does the name "CROSS_PROCESS"
at all discourage app authors from using it in their multi-threaded
multi-process apps. In most cases I would venture to guess that APR is
specifically built to be threaded or non-threaded depending on the mpm
we're using in APR (Caveat: what happens when APR becomes a standalone
library.)
Underlying all of this it is my belief that the single access point for
all thread types is causing this confusion in the interface, as well
as possibly inefficient and complicated implementations. But that is
another rant and I'll revisit that later. :)
> In certain cases where we know that the interprocess lock will do
> the job of the intraprocess lock, an optimization would be to
> forgo acquiring the extra intraprocess lock in the LOCK_ALL code.
> You may also forgo this intraprocess lock if APR isn't threaded.
> However, we brought up the pathological case of someone using a
> threaded APR with a non-threaded app. Is there something we can do
> here to optimize this case? Should we do something about this? (This
> seems to be what we have been discussing over the last day or so.)
>
> However, I don't see this optimization (or lack thereof) as a
> showstopper or causing any bugs. I do believe the current
> implementation of CROSS_PROCESS will cause bugs somewhere. And,
> while APR is fairly malleable, it'd be nice to get this right now
> rather than deal with cruft later.
>
> 2) Aaron and I were confused by the names used when examining the lock
> code. At first glance, CROSS_PROCESS and INTRAPROCESS aren't of the
> same format. Why isn't INTRAPROCESS - INTRA_PROCESS? Or,
> CROSS_PROCESS - CROSSPROCESS? Yes, this is completely semantic, but I
> think something needs to change (I'm a committer, so I guess I have some
> say in what gets changed!). I'd be happy enough to see INTERPROCESS
> and INTRAPROCESS. But, the POSIX naming style (PROCESS_SHARED and
> PROCESS_PRIVATE) might lead to less confusion from quick glances at
> code fragments (was that an ER or RA?).
agree++ (Can I do that? ;)
-aaron
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 03, 2001 at 03:07:30PM -0400, Jeff Trawick wrote:
> It is specific not just to the platform but to the mechanism.
>
> I wrote a simple APR app to test this out... I'll try to find time
> this evening to remove some bogosity, re-test, and post the app. I
> don't think the bogosity affects the outcome but I need to
> double-check.
But we can't run an APR app when we are in the middle of autoconf. =)
Don't we need to have this information before we compile? I'm sensing a
chicken and egg problem here. Please correct me if I'm wrong.
Backing up a bit to the original point of this thread. As I see it
there were two things initially addressed in Aaron's patch:
1) Removing CROSS_PROCESS and making it equivalent to LOCK_ALL.
On threaded APR, a CROSS_PROCESS lock may or may not implictly
acquire a intraprocess lock. I believe this is broken. We
shouldn't have a lock whose granularity is dependent upon the
platform and lock mechanism. In all cases that we examined (in
APR and httpd), the intention of the CROSS_PROCESS locks should
really be LOCK_ALL. Because some platforms/mechanisms will acquire
an INTRAPROCESS lock anyway with CROSS_PROCESS and others won't, the
current behavior seems ill-defined and should be standardized.
The LOCK_ALL behavior seems to be the correct one at the cost of a
potential extra lock acquisition where we weren't expecting the
INTRAPROCESS lock in the first place. My assumption would be
that this requesting this behavior (interprocess but not intraprocess)
would be very rare. I'd like to see a case where we would want to
lock out threads from other processes and not from our own process.
Furthermore, you can't satisfy this request anyway with the current
CROSS_PROCESS code as whether we obtain an intraprocess lock is
undefined. You can't assume that it'll happen.
In certain cases where we know that the interprocess lock will do
the job of the intraprocess lock, an optimization would be to
forgo acquiring the extra intraprocess lock in the LOCK_ALL code.
You may also forgo this intraprocess lock if APR isn't threaded.
However, we brought up the pathological case of someone using a
threaded APR with a non-threaded app. Is there something we can do
here to optimize this case? Should we do something about this? (This
seems to be what we have been discussing over the last day or so.)
However, I don't see this optimization (or lack thereof) as a
showstopper or causing any bugs. I do believe the current
implementation of CROSS_PROCESS will cause bugs somewhere. And,
while APR is fairly malleable, it'd be nice to get this right now
rather than deal with cruft later.
2) Aaron and I were confused by the names used when examining the lock
code. At first glance, CROSS_PROCESS and INTRAPROCESS aren't of the
same format. Why isn't INTRAPROCESS - INTRA_PROCESS? Or,
CROSS_PROCESS - CROSSPROCESS? Yes, this is completely semantic, but I
think something needs to change (I'm a committer, so I guess I have some
say in what gets changed!). I'd be happy enough to see INTERPROCESS
and INTRAPROCESS. But, the POSIX naming style (PROCESS_SHARED and
PROCESS_PRIVATE) might lead to less confusion from quick glances at
code fragments (was that an ER or RA?).
My $.02. -- justin
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@ebuilt.com> writes:
> On Tue, Jul 03, 2001 at 02:46:39PM -0400, Jeff Trawick wrote:
> > Note that APR currently doesn't have the right information now to set
> > APR_PROCESS_LOCK_MECH_IS_GLOBAL properly, so we end up getting two
> > locks instead of one on some platform/mechanisms where it isn't really
> > needed.
> >
> > But assuming that we get more hints or whatever to turn on
> > APR_PROCESS_LOCK_MECH_IS_GLOBAL in more cases then magic should
> > happen.
>
> How would we even go about determining this? Do we add this into
> apr_hints.m4 for each platform/OS that we know
> APR_PROCCESS_LOCK_MECH_IS_GLOBAL can be safely set, or can we
> somehow write an autoconf test for this? (I have no clue how we could
> write a test for this - this seems awfully complex). -- justin
note that APR_PROCESS_LOCK_MECH_IS_GLOBAL is not cool :( we need
something more granular, like
APR_SYSVSEM_IS_GLOBAL
APR_FLOCK_IS_GLOBAL
APR_FCNTL_IS_GLOBAL
etc.
It is specific not just to the platform but to the mechanism.
I wrote a simple APR app to test this out... I'll try to find time
this evening to remove some bogosity, re-test, and post the app. I
don't think the bogosity affects the outcome but I need to
double-check.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 03, 2001 at 02:46:39PM -0400, Jeff Trawick wrote:
> Note that APR currently doesn't have the right information now to set
> APR_PROCESS_LOCK_MECH_IS_GLOBAL properly, so we end up getting two
> locks instead of one on some platform/mechanisms where it isn't really
> needed.
>
> But assuming that we get more hints or whatever to turn on
> APR_PROCESS_LOCK_MECH_IS_GLOBAL in more cases then magic should
> happen.
How would we even go about determining this? Do we add this into
apr_hints.m4 for each platform/OS that we know
APR_PROCCESS_LOCK_MECH_IS_GLOBAL can be safely set, or can we
somehow write an autoconf test for this? (I have no clue how we could
write a test for this - this seems awfully complex). -- justin
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Justin Erenkrantz <je...@ebuilt.com> writes:
> [ Dropping new-httpd as this has no current relevance to them ]
>
> On Tue, Jul 03, 2001 at 09:25:26AM -0700, Aaron Bannert wrote:
> > That would be useful if APR_HAS_THREADS was somehow set per the app's
> > definition, not at APR's buildtime. I assume you're talking about this:
> >
> > case APR_PROCESS_SHARED:
> > if (new->inter_meth->flags & APR_PROCESS_LOCK_MECH_IS_GLOBAL) {
> > new->meth = new->inter_meth; /* impl. uses one lock */
> > }
> > else {
> > new->meth = &lockall_methods; /* impl. uses two locks */
> > }
> > break;
>
> When I looked at it this morning, I thought it didn't do what we want.
> Now, that I went back to sleep and woke up again, it seems to do what we
> want. That is defined when we are either on OS/390 or we don't have
> threads, which seems to be the correct definition. And, I guess that
> when we use pthread_mutex_t (which we think is broken) for
> PROCESS_SHARED, it automatically gets the PROCESS_PRIVATE lock.
Note that APR currently doesn't have the right information now to set
APR_PROCESS_LOCK_MECH_IS_GLOBAL properly, so we end up getting two
locks instead of one on some platform/mechanisms where it isn't really
needed.
But assuming that we get more hints or whatever to turn on
APR_PROCESS_LOCK_MECH_IS_GLOBAL in more cases then magic should
happen.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
[ Dropping new-httpd as this has no current relevance to them ]
On Tue, Jul 03, 2001 at 09:25:26AM -0700, Aaron Bannert wrote:
> That would be useful if APR_HAS_THREADS was somehow set per the app's
> definition, not at APR's buildtime. I assume you're talking about this:
>
> case APR_PROCESS_SHARED:
> if (new->inter_meth->flags & APR_PROCESS_LOCK_MECH_IS_GLOBAL) {
> new->meth = new->inter_meth; /* impl. uses one lock */
> }
> else {
> new->meth = &lockall_methods; /* impl. uses two locks */
> }
> break;
When I looked at it this morning, I thought it didn't do what we want.
Now, that I went back to sleep and woke up again, it seems to do what we
want. That is defined when we are either on OS/390 or we don't have
threads, which seems to be the correct definition. And, I guess that
when we use pthread_mutex_t (which we think is broken) for
PROCESS_SHARED, it automatically gets the PROCESS_PRIVATE lock.
So, I guess that your patch as currently posted will do the right thing
(only one lock) if we don't have threads. I wonder if this removes
Jeff's objections?
> > This could give the non-threaded users of APR even more reason to
> > specify "--without-threads" when configuring - which they should be
> > doing in the first place.
>
> yes. And we want recompiles of APR, no?
If you have a non-threaded app, you should use a non-threaded APR.
That's what makes sense to me, at least. -- justin
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Aaron Bannert <aa...@ebuilt.com>.
On Tue, Jul 03, 2001 at 07:55:50AM -0700, Justin Erenkrantz wrote:
> Wouldn't it be better to try and determine which platforms/lock types
> we don't need the APR_PROCESS_PRIVATE lock on when they attempt to
> acquire APR_PROCESS_SHARED? Is there some way to detect that?
That is what CROSS_PROCESS was for, but I argue that it is inappropriate
under other conditions (like threaded APR and threaded app).
> And, I believe we were saying that at least when you have threads,
> that the current behavior of CROSS_PROCESS is unacceptable (as it
> leaves the behavior on your process undefined which is completely
> bogus). So, for threaded APR, this is MUCH better, IMHO.
>
> For PROCESS_SHARED, I guess a compromise would be to do:
>
> create sharable mutex
> #if APR_HAS_THREADS
> if (sharable-mutex-type->flags & APR_SHARED_LOCK_DOES_NOT_LOCK_PRIVATE)
> create per-process mutex
> #endif
That would be useful if APR_HAS_THREADS was somehow set per the app's
definition, not at APR's buildtime. I assume you're talking about this:
case APR_PROCESS_SHARED:
if (new->inter_meth->flags & APR_PROCESS_LOCK_MECH_IS_GLOBAL) {
new->meth = new->inter_meth; /* impl. uses one lock */
}
else {
new->meth = &lockall_methods; /* impl. uses two locks */
}
break;
> I dunno. We know if we don't have threads that the PRIVATE mutex is not
> important. And, if our SHARED mutex also has the force of PRIVATE, then
> we don't even need to create the extra PRIVATE mutex. Or, maybe just
> always create the PRIVATE mutex if APR_HAS_THREADS is 1 (if we don't
> know if SHARED implies PRIVATE - which is our problem to begin with).
>
> This could give the non-threaded users of APR even more reason to
> specify "--without-threads" when configuring - which they should be
> doing in the first place.
yes. And we want recompiles of APR, no?
> (Man, I can't stand those god-awful CROSS_PROCESS names.) -- justin
(That's why I went for PROCESS_SHARED vs PROCESS_PRIVATE. "cross", "intra",
"inter", "blah" was used back and forth and was very confusing, especially
when "cross" isn't always truely "cross" since it's really just within
your process group, yadda yadda... :)
-aaron
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Aaron Bannert <aa...@ebuilt.com>.
On Tue, Jul 03, 2001 at 07:55:50AM -0700, Justin Erenkrantz wrote:
> Wouldn't it be better to try and determine which platforms/lock types
> we don't need the APR_PROCESS_PRIVATE lock on when they attempt to
> acquire APR_PROCESS_SHARED? Is there some way to detect that?
That is what CROSS_PROCESS was for, but I argue that it is inappropriate
under other conditions (like threaded APR and threaded app).
> And, I believe we were saying that at least when you have threads,
> that the current behavior of CROSS_PROCESS is unacceptable (as it
> leaves the behavior on your process undefined which is completely
> bogus). So, for threaded APR, this is MUCH better, IMHO.
>
> For PROCESS_SHARED, I guess a compromise would be to do:
>
> create sharable mutex
> #if APR_HAS_THREADS
> if (sharable-mutex-type->flags & APR_SHARED_LOCK_DOES_NOT_LOCK_PRIVATE)
> create per-process mutex
> #endif
That would be useful if APR_HAS_THREADS was somehow set per the app's
definition, not at APR's buildtime. I assume you're talking about this:
case APR_PROCESS_SHARED:
if (new->inter_meth->flags & APR_PROCESS_LOCK_MECH_IS_GLOBAL) {
new->meth = new->inter_meth; /* impl. uses one lock */
}
else {
new->meth = &lockall_methods; /* impl. uses two locks */
}
break;
> I dunno. We know if we don't have threads that the PRIVATE mutex is not
> important. And, if our SHARED mutex also has the force of PRIVATE, then
> we don't even need to create the extra PRIVATE mutex. Or, maybe just
> always create the PRIVATE mutex if APR_HAS_THREADS is 1 (if we don't
> know if SHARED implies PRIVATE - which is our problem to begin with).
>
> This could give the non-threaded users of APR even more reason to
> specify "--without-threads" when configuring - which they should be
> doing in the first place.
yes. And we want recompiles of APR, no?
> (Man, I can't stand those god-awful CROSS_PROCESS names.) -- justin
(That's why I went for PROCESS_SHARED vs PROCESS_PRIVATE. "cross", "intra",
"inter", "blah" was used back and forth and was very confusing, especially
when "cross" isn't always truely "cross" since it's really just within
your process group, yadda yadda... :)
-aaron
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 03, 2001 at 07:05:19AM -0400, Jeff Trawick wrote:
> Hmmm...
>
> A non-threaded app needs to serialize access to a resource across
> multiple processes.
>
> This non-threaded app builds/runs against a threaded APR.
Problem right there. You are going to be screwed performance-wise
anyway due to all of the needless mutex locking (PROCESS_PRIVATE aka
intraprocess) in the rest of the code.
> Before this patch:
>
> we only need one lock on many platforms (app specifies
> APR_CROSS_PROCESS)
>
> After this patch:
>
> we need two locks on many platforms (cross process and intra
> process)
>
> Isn't this going to bite prefork?
Wouldn't it be better to try and determine which platforms/lock types
we don't need the APR_PROCESS_PRIVATE lock on when they attempt to
acquire APR_PROCESS_SHARED? Is there some way to detect that?
And, I believe we were saying that at least when you have threads,
that the current behavior of CROSS_PROCESS is unacceptable (as it
leaves the behavior on your process undefined which is completely
bogus). So, for threaded APR, this is MUCH better, IMHO.
For PROCESS_SHARED, I guess a compromise would be to do:
create sharable mutex
#if APR_HAS_THREADS
if (sharable-mutex-type->flags & APR_SHARED_LOCK_DOES_NOT_LOCK_PRIVATE)
create per-process mutex
#endif
I dunno. We know if we don't have threads that the PRIVATE mutex is not
important. And, if our SHARED mutex also has the force of PRIVATE, then
we don't even need to create the extra PRIVATE mutex. Or, maybe just
always create the PRIVATE mutex if APR_HAS_THREADS is 1 (if we don't
know if SHARED implies PRIVATE - which is our problem to begin with).
This could give the non-threaded users of APR even more reason to
specify "--without-threads" when configuring - which they should be
doing in the first place.
(Man, I can't stand those god-awful CROSS_PROCESS names.) -- justin
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Aaron Bannert <aa...@ebuilt.com>.
Actually, I don't think an app would ever want to specify CROSS_PROCESS as
it is currently implemented, since it is not portable. How would an app
developer create a lock that will _never_ serialize in-process threads,
and _only_ synchronize across processes?
Maybe we're trying to mix too much into this part of APR? We have
threaded vs single, multiple lock scopes, multiple lock types, multiple
implementations on multiple platforms. All this under one set of
functions -- apr_lock_*(). (But this is a whole other discussion :).
You are correct that this will affect prefork, but only when built
against a threaded APR, and only on platforms where the crossplatform
lock mechanism is not global. And even then it will only mean that the
lock mechanism is implemented with two locks instead of one.
I see your point, but I see this is a tradeoff between API clarity and
crossplatform consistency, and a small amount of efficiency in isolated
cases.
-aaron
On Tue, Jul 03, 2001 at 07:05:19AM -0400, Jeff Trawick wrote:
> Hmmm...
>
> A non-threaded app needs to serialize access to a resource across
> multiple processes.
>
> This non-threaded app builds/runs against a threaded APR.
>
> Before this patch:
>
> we only need one lock on many platforms (app specifies
> APR_CROSS_PROCESS)
>
> After this patch:
>
> we need two locks on many platforms (cross process and intra
> process)
>
> Isn't this going to bite prefork?
>
> --
> Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Aaron Bannert <aa...@ebuilt.com>.
Actually, I don't think an app would ever want to specify CROSS_PROCESS as
it is currently implemented, since it is not portable. How would an app
developer create a lock that will _never_ serialize in-process threads,
and _only_ synchronize across processes?
Maybe we're trying to mix too much into this part of APR? We have
threaded vs single, multiple lock scopes, multiple lock types, multiple
implementations on multiple platforms. All this under one set of
functions -- apr_lock_*(). (But this is a whole other discussion :).
You are correct that this will affect prefork, but only when built
against a threaded APR, and only on platforms where the crossplatform
lock mechanism is not global. And even then it will only mean that the
lock mechanism is implemented with two locks instead of one.
I see your point, but I see this is a tradeoff between API clarity and
crossplatform consistency, and a small amount of efficiency in isolated
cases.
-aaron
On Tue, Jul 03, 2001 at 07:05:19AM -0400, Jeff Trawick wrote:
> Hmmm...
>
> A non-threaded app needs to serialize access to a resource across
> multiple processes.
>
> This non-threaded app builds/runs against a threaded APR.
>
> Before this patch:
>
> we only need one lock on many platforms (app specifies
> APR_CROSS_PROCESS)
>
> After this patch:
>
> we need two locks on many platforms (cross process and intra
> process)
>
> Isn't this going to bite prefork?
>
> --
> Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 03, 2001 at 07:05:19AM -0400, Jeff Trawick wrote:
> Hmmm...
>
> A non-threaded app needs to serialize access to a resource across
> multiple processes.
>
> This non-threaded app builds/runs against a threaded APR.
Problem right there. You are going to be screwed performance-wise
anyway due to all of the needless mutex locking (PROCESS_PRIVATE aka
intraprocess) in the rest of the code.
> Before this patch:
>
> we only need one lock on many platforms (app specifies
> APR_CROSS_PROCESS)
>
> After this patch:
>
> we need two locks on many platforms (cross process and intra
> process)
>
> Isn't this going to bite prefork?
Wouldn't it be better to try and determine which platforms/lock types
we don't need the APR_PROCESS_PRIVATE lock on when they attempt to
acquire APR_PROCESS_SHARED? Is there some way to detect that?
And, I believe we were saying that at least when you have threads,
that the current behavior of CROSS_PROCESS is unacceptable (as it
leaves the behavior on your process undefined which is completely
bogus). So, for threaded APR, this is MUCH better, IMHO.
For PROCESS_SHARED, I guess a compromise would be to do:
create sharable mutex
#if APR_HAS_THREADS
if (sharable-mutex-type->flags & APR_SHARED_LOCK_DOES_NOT_LOCK_PRIVATE)
create per-process mutex
#endif
I dunno. We know if we don't have threads that the PRIVATE mutex is not
important. And, if our SHARED mutex also has the force of PRIVATE, then
we don't even need to create the extra PRIVATE mutex. Or, maybe just
always create the PRIVATE mutex if APR_HAS_THREADS is 1 (if we don't
know if SHARED implies PRIVATE - which is our problem to begin with).
This could give the non-threaded users of APR even more reason to
specify "--without-threads" when configuring - which they should be
doing in the first place.
(Man, I can't stand those god-awful CROSS_PROCESS names.) -- justin
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Hmmm...
A non-threaded app needs to serialize access to a resource across
multiple processes.
This non-threaded app builds/runs against a threaded APR.
Before this patch:
we only need one lock on many platforms (app specifies
APR_CROSS_PROCESS)
After this patch:
we need two locks on many platforms (cross process and intra
process)
Isn't this going to bite prefork?
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Jeff Trawick <tr...@attglobal.net>.
Hmmm...
A non-threaded app needs to serialize access to a resource across
multiple processes.
This non-threaded app builds/runs against a threaded APR.
Before this patch:
we only need one lock on many platforms (app specifies
APR_CROSS_PROCESS)
After this patch:
we need two locks on many platforms (cross process and intra
process)
Isn't this going to bite prefork?
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] apr_lock.h update to remove/fix CROSS_PROCESS vs LOCKALL
Posted by Justin Erenkrantz <je...@ebuilt.com>.
+1. Makes the code so much easier to understand. -- justin
On Mon, Jul 02, 2001 at 11:30:59AM -0700, Aaron Bannert wrote:
> As discussed earlier on new-httpd and dev@apr, the lock scope
> CROSS_PROCESS is not portable, where LOCKALL is. I've fixed
> that by simply removing CROSS_PROCESS. The scopes we have left use names
> that are overloaded and confusing (LOCKALL vs CROSS_PROCESS huh?)
> so I went ahead and dropped the whole thing in favor of something
> more consistent and disambiguous, the POSIXified names:
>
> APR_PROCESS_PRIVATE -- (old INTRAPROCESS) synchronizes threads in the current
> process.
>
> APR_PROCESS_SHARED -- (old CROSS_PROCESS *and* LOCKALL) synchronizes threads
> across processes and their threads. Essentially acts as global lock
> on all platforms.
>
>
> The two patches must be applied in order (apr first, then httpd) since
> one depends on the other. The httpd changes were simple string
> replacements, but I looked them over before submitting. The apr changes
> seemed to work fine with the various apr/test/ routines on my platform
> (linux 2.4), but I don't have access to all the platforms that this
> patch might affect.
>
> -aaron