Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Apache: Dev

Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker

 

 

Apache dev RSS feed   Index | Next | Previous | View Threaded


trawick at gmail

Nov 23, 2009, 3:27 PM

Post #1 of 10 (964 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker

On Mon, Nov 23, 2009 at 6:17 PM, <trawick [at] apache> wrote:
> Author: trawick
> Date: Mon Nov 23 23:17:51 2009
> New Revision: 883540
>
> URL: http://svn.apache.org/viewvc?rev=883540&view=rev
> Log:
> Replace AcceptMutex, LockFile, RewriteLock, SSLMutex, SSLStaplingMutex,
> and WatchdogMutexPath with a single Mutex directive.  Add APIs to
> simplify setup and user customization of APR proc and global mutexes.
> (See util_mutex.h.)  Build-time setting DEFAULT_LOCKFILE is no longer
> respected; set DEFAULT_REL_RUNTIMEDIR instead.
>
> Some existing modules, such as mod_ldap and mod_auth_digest gain
> configurability for their mutexes.

There will likely be other issues to discuss about this, but to start with:

* you need an extremely recent checkout of apr-1.4.x in order to get
apr_global_mutex_lockfile()
* mod_watchdog hasn't been tested; what's a dead-easy way to make it
create a mutex?
* I haven't tested the change to mkconfNW.awk
* doc is still forthcoming


trawick at gmail

Nov 23, 2009, 3:59 PM

Post #2 of 10 (924 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

On Mon, Nov 23, 2009 at 6:33 PM, William A. Rowe Jr.
<wrowe [at] rowe-clan> wrote:
> trawick [at] apache wrote:
>> Author: trawick
>> Date: Mon Nov 23 23:17:51 2009
>> New Revision: 883540
>>
>> URL: http://svn.apache.org/viewvc?rev=883540&view=rev
>> Log:
>> Replace AcceptMutex, LockFile, RewriteLock, SSLMutex, SSLStaplingMutex,
>> and WatchdogMutexPath with a single Mutex directive.  Add APIs to
>> simplify setup and user customization of APR proc and global mutexes.
>> (See util_mutex.h.)  Build-time setting DEFAULT_LOCKFILE is no longer
>> respected; set DEFAULT_REL_RUNTIMEDIR instead.
>
> I haven't spent enough time follow the discussion thread, but there is a
> pretty big concern here.
>
> Let's say we pick NFS for our SSL session store, which means we need an fcntl
> or file lock to mutex the session cache.  But if these others were all machine
> local (and hopefully, easily threadproc mutexible, such as a pthread mutex or
> at worst case, a sysv sem) then the optimal no longer mirrors the appropriate
> match based on the resource.
>
> Can we perhaps partially revert to allow these to be -overrides- of the system
> wide mutex mechanism, e.g. sysv sem might be right for all but two resources.
> Let those two be overridden (and point out, in the docs, when this would become
> necessary).
>

I may not understand you, but here goes:

These are overrides of the compiled-in defaults (APR's default mutex
mech, httpd's default mutex file directory).
When you add

Mutex ssl-cache flock:/any/directory

that setting would override the compiled-in defaults just for that
mutex, and others would still be (e.g., on Linux) SysV semaphores.

If you then add

Mutex default posixsem

the ssl-cache mutex is still flock on the specified directory, but
everything else is a Posix semaphore.

What is missing for your scenario AFAICT is having a predictable lock
file name so that different nodes can share the same mutex file.
Currently on Unix the pid will always be appended to the lock file
name.

We could have an option on the ap_{global,proc}_mutex_create() or
ap_mutex_register() call to omit the pid from the lock file name.
Alternatively we could have an option on ap_mutex_register() to
indicate that the path specified on the Mutex directory, or in the
path parameter to ap_mutex_register(), is the absolute name of the
lock file.

Having this no-pid option hard-coded inside the module is a bit
hurtful, as getting the pid-qualified name by default avoids the
requirement for additional configuration when multiple httpd instances
on the same node share a lock directory.

Requiring that the full basename be coded on the Mutex directive is
hurtful since generally you just want to say "move it here", and in
fact most often you'd say "move them all here" by coding "Mutex
default file:/some/dir".

I guess something could be coded on the Mutex directive to indicate
that the given pathname is the entire lock file path for the given
mutex.

Make sense?


trawick at gmail

Nov 24, 2009, 3:52 AM

Post #3 of 10 (911 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

On Mon, Nov 23, 2009 at 6:59 PM, Jeff Trawick <trawick [at] gmail> wrote:
> On Mon, Nov 23, 2009 at 6:33 PM, William A. Rowe Jr.
> <wrowe [at] rowe-clan> wrote:
>> trawick [at] apache wrote:
>>> Author: trawick
>>> Date: Mon Nov 23 23:17:51 2009
>>> New Revision: 883540
>>>
>>> URL: http://svn.apache.org/viewvc?rev=883540&view=rev
>>> Log:
>>> Replace AcceptMutex, LockFile, RewriteLock, SSLMutex, SSLStaplingMutex,
>>> and WatchdogMutexPath with a single Mutex directive.  Add APIs to
>>> simplify setup and user customization of APR proc and global mutexes.
>>> (See util_mutex.h.)  Build-time setting DEFAULT_LOCKFILE is no longer
>>> respected; set DEFAULT_REL_RUNTIMEDIR instead.
>>
>> I haven't spent enough time follow the discussion thread, but there is a
>> pretty big concern here.
>>
>> Let's say we pick NFS for our SSL session store, which means we need an fcntl
>> or file lock to mutex the session cache.  But if these others were all machine
>> local (and hopefully, easily threadproc mutexible, such as a pthread mutex or
>> at worst case, a sysv sem) then the optimal no longer mirrors the appropriate
>> match based on the resource.
>>
>> Can we perhaps partially revert to allow these to be -overrides- of the system
>> wide mutex mechanism, e.g. sysv sem might be right for all but two resources.
>> Let those two be overridden (and point out, in the docs, when this would become
>> necessary).

> What is missing for your scenario AFAICT is having a predictable lock
> file name so that different nodes can share the same mutex file.
> Currently on Unix the pid will always be appended to the lock file
> name.

Interestingly, the mod_ssl doc currently states that the pid will be
appended automatically to the file specified in SSLMutex. You removed
the corresponding code in 2003 with the comment "It also removes the
mod_ssl historical '.pid' suffixes - that isn't how Apache2 specifies
files."


lists at glewis

Nov 24, 2009, 9:46 AM

Post #4 of 10 (906 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

Jeff Trawick wrote:
> On Mon, Nov 23, 2009 at 6:17 PM, <trawick [at] apache> wrote:
>> Author: trawick
>> Date: Mon Nov 23 23:17:51 2009
>> New Revision: 883540
>>
>> URL: http://svn.apache.org/viewvc?rev=883540&view=rev
> There will likely be other issues to discuss about this, but to start with:
>
> * you need an extremely recent checkout of apr-1.4.x in order to get
> apr_global_mutex_lockfile()

VC6 SP6 SDK 2003R2
VC9 SP1 SDK Included Express
Checkout APR 1.4 post r883540

> --- httpd/httpd/trunk/server/util_mutex.c 2009/03/26 12:44:48 758613
> +++ httpd/httpd/trunk/server/util_mutex.c 2009/11/23 23:17:51 883540
> @@ -31,7 +32,12 @@
> #include "httpd.h"
> #include "http_main.h"
> #include "http_config.h"
> +#include "http_log.h"
> #include "util_mutex.h"
> +#include "unixd.h"
> +#ifdef HAVE_UNISTD_H
> +#include <unistd.h> /* getpid() */
> +#endif

#include "unixd.h" in the open and anything associated with it is going
to bring Visual Studio to it's knees.

I see this in other places (socache modules at least)

#if AP_NEED_SET_MUTEX_PERMS
#include "unixd.h"
#endif


and maybe

/* Check for definition of DEFAULT_REL_RUNTIMEDIR */
#ifndef DEFAULT_REL_RUNTIMEDIR
#define DEFAULT_DBM_PREFIX "whatever_it_should_be"
#else
#define DEFAULT_DBM_PREFIX DEFAULT_REL_RUNTIMEDIR "whatever"
#endif

since it is not picking it up from mpm_default.h

VC6/9 doesn't seem too thrilled with this in core.c

AP_INIT_TAKE2("Mutex", ap_set_mutex, NULL, RSRC_CONF,
"mutex (or \"default\") and mechanism"),

VC6 E:\build\httpd-2.3.x-dev\server\core.c(3323) : error C2152:
'initializing' : pointers to functions with different attributes

VC9 .\server\core.c(3323) : error C2440: 'initializing' : cannot convert
from 'const char *(__stdcall *)(cmd_parms *,void *,const char *,const
char *)' to 'cmd_func'

not quite sure what it is telling me here other than possibly
ap_set_mutex is not returning a string, not sure.

Regards,
Gregg


trawick at gmail

Nov 24, 2009, 10:01 AM

Post #5 of 10 (915 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

On Tue, Nov 24, 2009 at 12:46 PM, Gregg L. Smith <lists [at] glewis> wrote:
> Jeff Trawick wrote:
>>
>> On Mon, Nov 23, 2009 at 6:17 PM,  <trawick [at] apache> wrote:
>>>
>>> Author: trawick
>>> Date: Mon Nov 23 23:17:51 2009
>>> New Revision: 883540
>>>
>>> URL: http://svn.apache.org/viewvc?rev=883540&view=rev
>>
>> There will likely be other issues to discuss about this, but to start
>> with:
>>
>> * you need an extremely recent checkout of apr-1.4.x in order to get
>> apr_global_mutex_lockfile()
>
> VC6 SP6 SDK 2003R2
> VC9 SP1 SDK Included Express
> Checkout APR 1.4 post r883540
>
>> --- httpd/httpd/trunk/server/util_mutex.c  2009/03/26 12:44:48  758613
>> +++ httpd/httpd/trunk/server/util_mutex.c  2009/11/23 23:17:51  883540
>> @@ -31,7 +32,12 @@
>>  #include "httpd.h"
>>  #include "http_main.h"
>>  #include "http_config.h"
>> +#include "http_log.h"
>>  #include "util_mutex.h"
>> +#include "unixd.h"
>> +#ifdef HAVE_UNISTD_H
>> +#include <unistd.h> /* getpid() */
>> +#endif
>
> #include "unixd.h" in the open and anything associated with it is going
> to bring Visual Studio to it's knees.

fixed in r883802

>
> I see this in other places (socache modules at least)
>
> #if AP_NEED_SET_MUTEX_PERMS
> #include "unixd.h"
> #endif
>

yep

>
> and maybe
>
> /* Check for definition of DEFAULT_REL_RUNTIMEDIR */
> #ifndef DEFAULT_REL_RUNTIMEDIR
> #define DEFAULT_DBM_PREFIX   "whatever_it_should_be"
> #else
> #define DEFAULT_DBM_PREFIX DEFAULT_REL_RUNTIMEDIR "whatever"
> #endif
>
> since it is not picking it up from mpm_default.h

I don't follow you on this one

>
> VC6/9 doesn't seem too thrilled with this in core.c
>
> AP_INIT_TAKE2("Mutex", ap_set_mutex, NULL, RSRC_CONF,
>              "mutex (or \"default\") and mechanism"),
>
> VC6 E:\build\httpd-2.3.x-dev\server\core.c(3323) : error C2152:
> 'initializing' : pointers to functions with different attributes
>
> VC9 .\server\core.c(3323) : error C2440: 'initializing' : cannot convert
> from 'const char *(__stdcall *)(cmd_parms *,void *,const char *,const
> char *)' to 'cmd_func'
>
> not quite sure what it is telling me here other than possibly ap_set_mutex
> is not returning a string, not sure.

calling convention incompatibility; I guess the command function
macros have hard-coded a particular calling convention that is
different from our API calling convent (AP_DECLARE())

can you try the attached patch pretty please?

Thanks!
Attachments: win-api-fix.txt (1.31 KB)


lists at glewis

Nov 24, 2009, 11:01 AM

Post #6 of 10 (912 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

Jeff Trawick wrote:
> On Tue, Nov 24, 2009 at 12:46 PM, Gregg L. Smith <lists [at] glewis> wrote:
>> Jeff Trawick wrote:
>> and maybe
>>
>> /* Check for definition of DEFAULT_REL_RUNTIMEDIR */
>> #ifndef DEFAULT_REL_RUNTIMEDIR
>> #define DEFAULT_DBM_PREFIX "whatever_it_should_be"
>> #else
>> #define DEFAULT_DBM_PREFIX DEFAULT_REL_RUNTIMEDIR "whatever"
>> #endif
>>
>> since it is not picking it up from mpm_default.h
>
> I don't follow you on this one

Ok, let me express it this way.

in util_mutex.c line 160 you've got this;

def->dir = DEFAULT_REL_RUNTIMEDIR;

The only place I find DEFAULT_REL_RUNTIMEDIR defined is in mpm_default.h
You do not include mpm_default.h most likely since it would introduce a
lot of excess baggage, understandable. So if you do not want to include
the excess baggage, it must defind in the util_mutex.c no?

Similar again to the socache modules which I stole that code snip from.
I'm just not sure what the "whatever_it_should_be" and "whatever" should
actually be.

In server/mpm/winnt/mpm_default.h it's just the one "logs" (lines 64-67)

/* Check for definition of DEFAULT_REL_RUNTIMEDIR */
#ifndef DEFAULT_REL_RUNTIMEDIR
#define DEFAULT_REL_RUNTIMEDIR "logs"
#endif

However in mod_socache_dbm.c (lines 56-61) for example it's

/* Check for definition of DEFAULT_REL_RUNTIMEDIR */
#ifndef DEFAULT_REL_RUNTIMEDIR
#define DEFAULT_DBM_PREFIX "logs/socache-dbm-"
#else
#define DEFAULT_DBM_PREFIX DEFAULT_REL_RUNTIMEDIR "/socache-dbm-"
#endif

Do you follow me now?

There's a similar 'use before checking if it's defined' in mod_ldap as
well but that's another issue that does not concern this.

> calling convention incompatibility; I guess the command function
> macros have hard-coded a particular calling convention that is
> different from our API calling convent (AP_DECLARE())
>
> can you try the attached patch pretty please?

most certainly can and did and the build passes that point and she will
at least serve up a "It works!".

Stoopid Windoze ;-)
Gregg


lists at glewis

Nov 24, 2009, 11:24 AM

Post #7 of 10 (908 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

Maybe I should add, build will fail on line util_mutex.c line 160
unless DEFAULT_REL_RUNTIMEDIR is defined, whether by also including
mpm_default.h or adding a define in util_mutex.c or
util_mutex.h.

E:\build\httpd-2.3.x-dev\server\util_mutex.c(159) : error C2065:
'DEFAULT_REL_RUNTIMEDIR' : undeclared identifier


Sorry for the lack of clarity again.
Gregg


Gregg L. Smith wrote:
> Jeff Trawick wrote:
>> On Tue, Nov 24, 2009 at 12:46 PM, Gregg L. Smith <lists [at] glewis>
>> wrote:
>>> Jeff Trawick wrote:
> Ok, let me express it this way.
>
> in util_mutex.c line 160 you've got this;
>
> def->dir = DEFAULT_REL_RUNTIMEDIR;
>
> The only place I find DEFAULT_REL_RUNTIMEDIR defined is in mpm_default.h
> You do not include mpm_default.h most likely since it would introduce a
> lot of excess baggage, understandable. So if you do not want to include
> the excess baggage, it must defind in the util_mutex.c no?
>
> Similar again to the socache modules which I stole that code snip from.
> I'm just not sure what the "whatever_it_should_be" and "whatever" should
> actually be.
>
> In server/mpm/winnt/mpm_default.h it's just the one "logs" (lines 64-67)
>
> /* Check for definition of DEFAULT_REL_RUNTIMEDIR */
> #ifndef DEFAULT_REL_RUNTIMEDIR
> #define DEFAULT_REL_RUNTIMEDIR "logs"
> #endif
>
> However in mod_socache_dbm.c (lines 56-61) for example it's
>
> /* Check for definition of DEFAULT_REL_RUNTIMEDIR */
> #ifndef DEFAULT_REL_RUNTIMEDIR
> #define DEFAULT_DBM_PREFIX "logs/socache-dbm-"
> #else
> #define DEFAULT_DBM_PREFIX DEFAULT_REL_RUNTIMEDIR "/socache-dbm-"
> #endif
>
> Do you follow me now?
>
> There's a similar 'use before checking if it's defined' in mod_ldap as
> well but that's another issue that does not concern this.
>
>> calling convention incompatibility; I guess the command function
>> macros have hard-coded a particular calling convention that is
>> different from our API calling convent (AP_DECLARE())
>>
>> can you try the attached patch pretty please?
>
> most certainly can and did and the build passes that point and she will
> at least serve up a "It works!".
>
> Stoopid Windoze ;-)
> Gregg
>
>
>
>
>
>


trawick at gmail

Nov 24, 2009, 11:31 AM

Post #8 of 10 (909 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

On Tue, Nov 24, 2009 at 2:24 PM, Gregg L. Smith <lists [at] glewis> wrote:
> Maybe I should add, build will fail on line util_mutex.c line 160
> unless DEFAULT_REL_RUNTIMEDIR is defined, whether by also including
> mpm_default.h or adding a define in util_mutex.c or
> util_mutex.h.
>
> E:\build\httpd-2.3.x-dev\server\util_mutex.c(159) : error C2065:
> 'DEFAULT_REL_RUNTIMEDIR' : undeclared identifier
>
>
> Sorry for the lack of clarity again.

no, even I could understand

I fixed it by hardcoding "logs" when DEFAULT_REL_RUNTIMEDIR isn't defined.
(mpm_default.h won't be found on Unix without build changes, and we
want to minimize affinity with any particular MPM anyway.)

I left a note to clean up access to DEFAULT_REL_RUNTIMEDIR for some
other rainy day.


lists at glewis

Nov 24, 2009, 12:12 PM

Post #9 of 10 (913 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

Jeff Trawick wrote:
> no, even I could understand
>
> I fixed it by hardcoding "logs" when DEFAULT_REL_RUNTIMEDIR isn't defined.

This works for me at least.

> (mpm_default.h won't be found on Unix without build changes, and we
> want to minimize affinity with any particular MPM anyway.)
>

I find that odd since I see it in the mpm_default.h files under all the
various mpms (worker/simple/event).

> I left a note to clean up access to DEFAULT_REL_RUNTIMEDIR for some
> other rainy day.
>

Rainy days are for such things. I see wrowe's comment post your reply,
do you feel like a ping-pong ball yet? ;-)

I thank you for your prompt attention to this matter,
Gregg


trawick at gmail

Nov 24, 2009, 12:37 PM

Post #10 of 10 (912 views)
Permalink
Re: svn commit: r883540 - in /httpd/httpd/trunk: ./ build/ docs/conf/ docs/conf/extra/ include/ modules/aaa/ modules/core/ modules/examples/ modules/generators/ modules/ldap/ modules/mappers/ modules/ssl/ server/ server/mpm/prefork/ server/mpm/worker [In reply to]

On Tue, Nov 24, 2009 at 3:12 PM, Gregg L. Smith <lists [at] glewis> wrote:
> Jeff Trawick wrote:
>>
>> (mpm_default.h won't be found on Unix without build changes, and we
>> want to minimize affinity with any particular MPM anyway.)
>>
>
> I find that odd since I see it in the mpm_default.h files under all the
> various mpms (worker/simple/event).

The other MPMs use that setting internally, but don't define that setting.

>I see wrowe's comment post your reply, do
> you feel like a ping-pong ball yet? ;-)

yes!

Apache dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.