HTTP tests fail on OS X

HTTP tests fail on OS X

[Brian Gernhardt]

I've bisected it to this commit:

> commit 0442743810c6f6c14386a5a9d6bf8e4d69adbc51
> Author: Jeff King <peff@peff.net>
> Date:   Sun Jun 9 04:07:59 2013 -0400
> 
>     t/lib-httpd/apache.conf: do not use LockFile in apache >= 2.4

OS X 10.8.4, apache 2.2.22

I'll also note that the tests fail the first time they attempt to access the server and the cleanup function notes that httpd is not running under the expected PID.  Looking at the httpd setup code, I would have expected to see errors (with -v) and have the setup fail but neither happens.

It's way too late for me, so I can't look at it again for at least several hours.  I figured I'd ping the list in case the problem/solution is obvious to someone else.

~~ Brian Gernhardt

Re: HTTP tests fail on OS X

[Jeff King]

On Fri, Jun 21, 2013 at 12:27:02AM -0400, Brian Gernhardt wrote:

> I've bisected it to this commit:
> 
> > commit 0442743810c6f6c14386a5a9d6bf8e4d69adbc51
> > Author: Jeff King <peff@peff.net>
> > Date:   Sun Jun 9 04:07:59 2013 -0400
> > 
> >     t/lib-httpd/apache.conf: do not use LockFile in apache >= 2.4
> 
> OS X 10.8.4, apache 2.2.22

Hrm. That's quite odd. The patch in its entirety looks like this:

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 542241b..891edd7 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -1,5 +1,7 @@ LockFile accept.lock
 ServerName dummy
+<IfVersion < 2.4>
 LockFile accept.lock
+</IfVersion>
 PidFile httpd.pid
 DocumentRoot www

So it should not be having any effect at all on apache 2.2, unless it
somehow does not like the <IfVersion> tags.

Puzzled...

-Peff

Re: HTTP tests fail on OS X

[Jeff King]

On Fri, Jun 21, 2013 at 12:30:52AM -0400, Jeff King wrote:

> > OS X 10.8.4, apache 2.2.22
> 
> Hrm. That's quite odd. The patch in its entirety looks like this:
> [...]
> So it should not be having any effect at all on apache 2.2, unless it
> somehow does not like the <IfVersion> tags.

Just to double-check, I ran all of the http tests in "next" (with my
series) successfully on apache 2.2.16. So the good news is I haven't
broken apache 2.2.x entirely. :)

I'm not sure if there is something different between 2.2.16 and 2.2.22,
or something with the particular build. Here's my -V output, in case it
helps:

    $ apache2 -V
    Server version: Apache/2.2.16 (Debian)
    Server built:   Mar  3 2013 12:12:28
    Server's Module Magic Number: 20051115:24
    Server loaded:  APR 1.4.2, APR-Util 1.3.9
    Compiled using: APR 1.4.2, APR-Util 1.3.9
    Architecture:   64-bit
    Server MPM:     Worker
      threaded:     yes (fixed thread count)
        forked:     yes (variable process count)
    Server compiled with....
     -D APACHE_MPM_DIR="server/mpm/worker"
     -D APR_HAS_SENDFILE
     -D APR_HAS_MMAP
     -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
     -D APR_USE_SYSVSEM_SERIALIZE
     -D APR_USE_PTHREAD_SERIALIZE
     -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
     -D APR_HAS_OTHER_CHILD
     -D AP_HAVE_RELIABLE_PIPED_LOGS
     -D DYNAMIC_MODULE_LIMIT=128
     -D HTTPD_ROOT="/etc/apache2"
     -D SUEXEC_BIN="/usr/lib/apache2/suexec"
     -D DEFAULT_PIDLOG="/var/run/apache2.pid"
     -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
     -D DEFAULT_ERRORLOG="logs/error_log"
     -D AP_TYPES_CONFIG_FILE="mime.types"
     -D SERVER_CONFIG_FILE="apache2.conf"

-Peff

Re: HTTP tests fail on OS X

[Brian Gernhardt]

On Jun 21, 2013, at 12:42 AM, Jeff King <peff@peff.net> wrote:

> I'm not sure if there is something different between 2.2.16 and 2.2.22,
> or something with the particular build. Here's my -V output, in case it
> helps:
> 
>    $ apache2 -V
>    Server version: Apache/2.2.16 (Debian)
>    Server built:   Mar  3 2013 12:12:28
>    Server's Module Magic Number: 20051115:24
>    Server loaded:  APR 1.4.2, APR-Util 1.3.9
>    Compiled using: APR 1.4.2, APR-Util 1.3.9
>    Architecture:   64-bit
>    Server MPM:     Worker
>      threaded:     yes (fixed thread count)
>        forked:     yes (variable process count)
>    Server compiled with....
>     -D APACHE_MPM_DIR="server/mpm/worker"
>     -D APR_HAS_SENDFILE
>     -D APR_HAS_MMAP
>     -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
>     -D APR_USE_SYSVSEM_SERIALIZE
>     -D APR_USE_PTHREAD_SERIALIZE
>     -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
>     -D APR_HAS_OTHER_CHILD
>     -D AP_HAVE_RELIABLE_PIPED_LOGS
>     -D DYNAMIC_MODULE_LIMIT=128
>     -D HTTPD_ROOT="/etc/apache2"
>     -D SUEXEC_BIN="/usr/lib/apache2/suexec"
>     -D DEFAULT_PIDLOG="/var/run/apache2.pid"
>     -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
>     -D DEFAULT_ERRORLOG="logs/error_log"
>     -D AP_TYPES_CONFIG_FILE="mime.types"
>     -D SERVER_CONFIG_FILE="apache2.conf"

Doesn't look terribly different from mine.

$ apachectl -V
Server version: Apache/2.2.22 (Unix)
Server built:   Dec  9 2012 18:57:18
Server's Module Magic Number: 20051115:30
Server loaded:  APR 1.4.5, APR-Util 1.3.12
Compiled using: APR 1.4.5, APR-Util 1.3.12
Architecture:   64-bit
Server MPM:     Prefork
  threaded:     no
    forked:     yes (variable process count)
Server compiled with....
 -D APACHE_MPM_DIR="server/mpm/prefork"
 -D APR_HAS_SENDFILE
 -D APR_HAS_MMAP
 -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
 -D APR_USE_FLOCK_SERIALIZE
 -D APR_USE_PTHREAD_SERIALIZE
 -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
 -D APR_HAS_OTHER_CHILD
 -D AP_HAVE_RELIABLE_PIPED_LOGS
 -D DYNAMIC_MODULE_LIMIT=128
 -D HTTPD_ROOT="/usr"
 -D SUEXEC_BIN="/usr/bin/suexec"
 -D DEFAULT_PIDLOG="/private/var/run/httpd.pid"
 -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
 -D DEFAULT_LOCKFILE="/private/var/run/accept.lock"
 -D DEFAULT_ERRORLOG="logs/error_log"
 -D AP_TYPES_CONFIG_FILE="/private/etc/apache2/mime.types"
 -D SERVER_CONFIG_FILE="/private/etc/apache2/httpd.conf"

Re: HTTP tests fail on OS X

[Jeff King]

On Fri, Jun 21, 2013 at 12:45:25AM -0400, Brian Gernhardt wrote:

> > I'm not sure if there is something different between 2.2.16 and 2.2.22,
> > or something with the particular build. Here's my -V output, in case it
> > helps:
> [...]
> Doesn't look terribly different from mine.

It turns out that 2.2.22 is in Debian testing, so it was easy for me to
downgrade from 2.4 and try it. Seems to also work fine for me. So I
don't think it's an apache version issue.

I also installed the prefork mpm in case that was part of the problem.
But it all seems fine here, and my build parameters look quite similar
to yours.

I'm not sure what else to look at...I guess try ratcheting up the
debugging/log level on your failing copy and see if it prints anything
useful.

-Peff

Re: HTTP tests fail on OS X

[Brian Gernhardt]

On Jun 21, 2013, at 12:49 AM, Jeff King <peff@peff.net> wrote:

> I'm not sure what else to look at...I guess try ratcheting up the
> debugging/log level on your failing copy and see if it prints anything
> useful.

I found this error in the error.log:

[Fri Jun 21 12:59:59 2013] [emerg] (2)No such file or directory: Couldn't create accept lock (/private/var/run/accept.lock.64288) (5)

Annoying that httpd returns before it tries to create the lock.

Without the IfVersion directive, it creates the lock in the ./httpd directory instead.  Not sure why apache is doing that, it's very irritating.

~~ Brian

Re: HTTP tests fail on OS X

[Jeff King]

On Fri, Jun 21, 2013 at 01:03:40PM -0400, Brian Gernhardt wrote:

> On Jun 21, 2013, at 12:49 AM, Jeff King <peff@peff.net> wrote:
> 
> > I'm not sure what else to look at...I guess try ratcheting up the
> > debugging/log level on your failing copy and see if it prints anything
> > useful.
> 
> I found this error in the error.log:
> 
> [Fri Jun 21 12:59:59 2013] [emerg] (2)No such file or directory: Couldn't create accept lock (/private/var/run/accept.lock.64288) (5)

Hmm. I am far from an apache expert, but I believe that is what would
happen if the LockFile directive was not there at all. IOW, it seems
like your apache is treating "<IfVersion < 2.4>" as false. Which seems
weird to me.

It's possible I'm using IfVersion wrong, though it does seem to work for
me elsewhere.

IfVersion comes from mod_version. I assume that if it were not loaded,
apache would complain about the directive entirely. But it's true that
we don't load it until later. Maybe try moving the IfVersion/Lockfile
stanza down below the mod_version LoadModule line?

-Peff

Re: HTTP tests fail on OS X

[Brian Gernhardt]

On Jun 21, 2013, at 2:03 PM, Jeff King <peff@peff.net> wrote:

> IfVersion comes from mod_version. I assume that if it were not loaded,
> apache would complain about the directive entirely. But it's true that
> we don't load it until later. Maybe try moving the IfVersion/Lockfile
> stanza down below the mod_version LoadModule line?

Apache is apparently overly accepting.  Moving the IfVersion below all the IfModules fixes it.

~~ Brian

[PATCH] lib-httpd/apache.conf: check version only after mod_version loads

[Jeff King]

On Fri, Jun 21, 2013 at 02:08:49PM -0400, Brian Gernhardt wrote:

> On Jun 21, 2013, at 2:03 PM, Jeff King <peff@peff.net> wrote:
> 
> > IfVersion comes from mod_version. I assume that if it were not
> > loaded, apache would complain about the directive entirely. But it's
> > true that we don't load it until later. Maybe try moving the
> > IfVersion/Lockfile stanza down below the mod_version LoadModule
> > line?
> 
> Apache is apparently overly accepting.  Moving the IfVersion below all
> the IfModules fixes it.

Cool. I think the patch should look like the one below, then.

Just to double-check that I have explained the issue correctly, can you
share the output of "apache2 -l"? Mine has:

  $ apache2 -l
  Compiled in modules:
    core.c
    mod_log_config.c
    mod_logio.c
    mod_version.c
    prefork.c
    http_core.c
    mod_so.c

which explains why it works here. I'm assuming you will not have
mod_version.c compiled in.

-- >8 --
Subject: lib-httpd/apache.conf: check version only after mod_version loads

Commit 0442743 introduced an <IfVersion> directive near the
top of the apache config file. However, at that point we
have not yet checked for and loaded the mod_version module.
This means that the directive will behave oddly if
mod_version is dynamically loaded, failing to match when it
should.

We can fix this by moving the whole block below the
LoadModule directive for mod_version.

Reported-by: Brian Gernhardt <brian@gernhardtsoftware.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/lib-httpd/apache.conf | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 56ae548..dd17e3a 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -1,7 +1,4 @@ ServerName dummy
 ServerName dummy
-<IfVersion < 2.4>
-LockFile accept.lock
-</IfVersion>
 PidFile httpd.pid
 DocumentRoot www
 LogFormat "%h %l %u %t \"%r\" %>s %b" common
@@ -26,6 +23,10 @@ ErrorLog error.log
 	LoadModule version_module modules/mod_version.so
 </IfModule>
 
+<IfVersion < 2.4>
+LockFile accept.lock
+</IfVersion>
+
 <IfVersion < 2.1>
 <IfModule !mod_auth.c>
 	LoadModule auth_module modules/mod_auth.so
-- 
1.8.3.rc2.14.g7eee6b3

Re: [PATCH] lib-httpd/apache.conf: check version only after mod_version loads

[Brian Gernhardt]

On Jun 21, 2013, at 2:12 PM, Jeff King <peff@peff.net> wrote:

> On Fri, Jun 21, 2013 at 02:08:49PM -0400, Brian Gernhardt wrote:
> 
>> On Jun 21, 2013, at 2:03 PM, Jeff King <peff@peff.net> wrote:
>> 
>>> IfVersion comes from mod_version. I assume that if it were not
>>> loaded, apache would complain about the directive entirely. But it's
>>> true that we don't load it until later. Maybe try moving the
>>> IfVersion/Lockfile stanza down below the mod_version LoadModule
>>> line?
>> 
>> Apache is apparently overly accepting.  Moving the IfVersion below all
>> the IfModules fixes it.
> 
> Cool. I think the patch should look like the one below, then.

Basically identical to what I've done, you're just faster to the actual patch.  :-D

+1

> Just to double-check that I have explained the issue correctly, can you
> share the output of "apache2 -l"? Mine has:
> 
>  $ apache2 -l
>  Compiled in modules:
>    core.c
>    mod_log_config.c
>    mod_logio.c
>    mod_version.c
>    prefork.c
>    http_core.c
>    mod_so.c
> 
> which explains why it works here. I'm assuming you will not have
> mod_version.c compiled in.

Indeed I do not.

 $ httpd -l
 Compiled in modules:
   core.c
   prefork.c
   http_core.c
   mod_so.c

Re: [PATCH] lib-httpd/apache.conf: check version only after mod_version loads

[Jeff King]

On Fri, Jun 21, 2013 at 02:15:39PM -0400, Brian Gernhardt wrote:

> > Cool. I think the patch should look like the one below, then.
> 
> Basically identical to what I've done, you're just faster to the actual patch.  :-D

I started writing the rationale immediately after making the suggestion,
before you responded.  Lucky for me my guess turned out to be right. :)

> > which explains why it works here. I'm assuming you will not have
> > mod_version.c compiled in.
> 
> Indeed I do not.
> 
>  $ httpd -l
>  Compiled in modules:
>    core.c
>    prefork.c
>    http_core.c
>    mod_so.c

Makes sense. Thanks for the bug report.

-Peff

Re: [PATCH] lib-httpd/apache.conf: check version only after mod_version loads

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Cool. I think the patch should look like the one below, then.
>
> Just to double-check that I have explained the issue correctly, can you
> share the output of "apache2 -l"? Mine has:
>
>   $ apache2 -l
>   Compiled in modules:
>     core.c
>     mod_log_config.c
>     mod_logio.c
>     mod_version.c
>     prefork.c
>     http_core.c
>     mod_so.c
>
> which explains why it works here. I'm assuming you will not have
> mod_version.c compiled in.
>
> -- >8 --
> Subject: lib-httpd/apache.conf: check version only after mod_version loads
>
> Commit 0442743 introduced an <IfVersion> directive near the
> top of the apache config file. However, at that point we
> have not yet checked for and loaded the mod_version module.
> This means that the directive will behave oddly if
> mod_version is dynamically loaded, failing to match when it
> should.
>
> We can fix this by moving the whole block below the
> LoadModule directive for mod_version.
>
> Reported-by: Brian Gernhardt <brian@gernhardtsoftware.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/lib-httpd/apache.conf | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index 56ae548..dd17e3a 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -1,7 +1,4 @@ ServerName dummy
>  ServerName dummy
> -<IfVersion < 2.4>
> -LockFile accept.lock
> -</IfVersion>
>  PidFile httpd.pid
>  DocumentRoot www
>  LogFormat "%h %l %u %t \"%r\" %>s %b" common
> @@ -26,6 +23,10 @@ ErrorLog error.log
>  	LoadModule version_module modules/mod_version.so
>  </IfModule>
>  
> +<IfVersion < 2.4>
> +LockFile accept.lock
> +</IfVersion>
> +

Once you see it in the patch form, it is very clear what this change
does and why it is necessary in the context ;-)

Thanks, both of you, for digging this down to the root cause; you
guys have done before it graduates to 'master', which I especially
appreciate.

Will queue.