You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@covalent.net on 2000/04/26 19:26:19 UTC
Re: [PATCH] config tree bug (is anybody hitting one)???
I'm not hitting this bug, and I've been exercising the config stuff pretty
hard recently. Are you using the default config?
Ryan
On Wed, 26 Apr 2000, Jeff Trawick wrote:
> I'm not sure why nobody else is hitting this. This would seem to affect
> many config files. (The obvious answer is that I've screwed up
> somewhere, but I'll choose to ignore that possibility for the moment :) )
>
> For a config file that starts out as:
>
> <IfModule prefork.c>
> StartServers 10
> MinSpareServers 5
> MaxSpareServers 40
> MaxClients 200
> MaxRequestsPerChild 0
> </IfModule>
>
> I'm getting this init failure:
>
> [trawick@k5 src]$ ./httpd -f /tmp/apache-2.0/conf/tmp.conf
> Syntax error on line 7 of /tmp/apache-2.0/conf/tmp.conf:
> </IfModule> without matching <IfModule> section
>
> ap_build_config_sub() is confused because curr_parent is not set
> properly.
>
> With the following patch it goes away. I'm not confident that
> this is the correct code or the correct place for it, but perhaps Ryan
> or Greg Stein will know.
>
> Index: main/http_config.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/main/http_config.c,v
> retrieving revision 1.44
> diff -u -r1.44 http_config.c
> --- http_config.c 2000/04/26 07:14:32 1.44
> +++ http_config.c 2000/04/26 19:53:59
> @@ -976,6 +976,9 @@
>
> if (*conftree == NULL && current != NULL) {
> *conftree = current;
> + if (!curr_parent) {
> + curr_parent = current;
> + }
> }
> }
>
> --
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Jeff Trawick <tr...@bellsouth.net>.
> We have a similar problem with this cfg file:
>
> Servername gobble
> <Directory />
> <IfModule prefork.c>
> gobble gobble
> </IfModule>
> </Directory>
>
> current/curr_parent don't get set properly when starting on <IfModule
> prefork.c>. If I stick some statement before <IfModule prefork.c> it
> parses o.k.
>
> So...
>
> problem 1: current/curr_parent not set properly when the first thing
> in the config file is a container (fixed by my last patch)
>
> problem 2: current/curr_parent not set properly when the first thing
> in a container is a container (not fixed by my last patch)
>
> Hopefully something more general than my last patch will cover both
> problems.
I can't say that this is more general, but it does fix problem 2
(problem 1 is still fixed).
Index: main/http_config.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/main/http_config.c,v
retrieving revision 1.44
diff -u -r1.44 http_config.c
--- http_config.c 2000/04/26 07:14:32 1.44
+++ http_config.c 2000/04/27 17:54:06
@@ -865,6 +865,7 @@
if (cmd_name[0] == '<') {
if (cmd_name[1] != '/') {
+ newdir->container = 1;
(*current) = ap_add_node(curr_parent, *current, newdir, 1);
}
else if (*curr_parent == NULL) {
@@ -976,6 +977,12 @@
if (*conftree == NULL && current != NULL) {
*conftree = current;
+ if (!curr_parent) {
+ if (current->container) {
+ curr_parent = current;
+ current = NULL;
+ }
+ }
}
}
Index: main/util_cfgtree.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/main/util_cfgtree.c,v
retrieving revision 1.3
diff -u -r1.3 util_cfgtree.c
--- util_cfgtree.c 2000/04/22 22:54:30 1.3
+++ util_cfgtree.c 2000/04/27 17:54:06
@@ -69,7 +69,13 @@
(*parent)->first_child = toadd;
toadd->parent = *parent;
}
- return toadd;
+ if (toadd->container) {
+ *parent = toadd;
+ return NULL;
+ }
+ else {
+ return toadd;
+ }
}
current->next = toadd;
toadd->parent = *parent;
> Paul, did you narrow down your problem to a small set of config
> statements that you can post?
Anybody with rejected (minimal) configs they want to post?
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Jeff Trawick <tr...@bellsouth.net>.
We have a similar problem with this cfg file:
Servername gobble
<Directory />
<IfModule prefork.c>
gobble gobble
</IfModule>
</Directory>
current/curr_parent don't get set properly when starting on <IfModule
prefork.c>. If I stick some statement before <IfModule prefork.c> it
parses o.k.
So...
problem 1: current/curr_parent not set properly when the first thing
in the config file is a container (fixed by my last patch)
problem 2: current/curr_parent not set properly when the first thing
in a container is a container (not fixed by my last patch)
Hopefully something more general than my last patch will cover both
problems.
Paul, did you narrow down your problem to a small set of config
statements that you can post?
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by "Paul J. Reder" <re...@raleigh.ibm.com>.
Ryan,
That patch is not fixing the processing for my config file. I am
currently trying to narrow the problem down to a smaller set of
directives. I will post the file as soon as I have it narrowed down.
rbb@covalent.net wrote:
>
> On Thu, 27 Apr 2000 rbb@covalent.net wrote:
>
> >
> >
> > The problem is that the tree is no longer being built correctly. This
> > patch does not fix the underlying problem. I am looking into this right
> > now.
>
> I should review e-mail much closer. It looks like this patch is taking
> care of the problem. DOH!
>
> I'll review a bit closer and let you know.
>
> Ryan
--
Paul J. Reder
---------------------------------------------------------------------
Noise proves nothing. Often a hen who has merely laid an egg cackles
as if she laid an asteroid.
-- Mark Twain
(An apt quote in this political season.)
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by rb...@covalent.net.
On Thu, 27 Apr 2000 rbb@covalent.net wrote:
>
>
> The problem is that the tree is no longer being built correctly. This
> patch does not fix the underlying problem. I am looking into this right
> now.
I should review e-mail much closer. It looks like this patch is taking
care of the problem. DOH!
I'll review a bit closer and let you know.
Ryan
>
> Ryan
>
> On Thu, 27 Apr 2000, Jeff Trawick wrote:
>
> > Here is a patch that works for a larger set of cfg files :)
> >
> > The problem (or one problem) seems to be that curr_parent/current
> > don't get set up properly when the first element of the cfg file is a
> > container.
> >
> > Maybe these changes will give you some hints. With these changes,
> > this config file looks real pretty in ddd:
> >
> > <IfModule prefork.c>
> > MinSpareServers 20
> > MaxSpareServers 40
> > </IfModule>
> > servername gobble
> >
> > Without the changes, MinSpareServers is pointed to by the next pointer
> > of "<IfModule prefork.c>" and "</IfModule>" is supposedly dangling.
> >
> > Index: main/http_config.c
> > ===================================================================
> > RCS file: /cvs/apache/apache-2.0/src/main/http_config.c,v
> > retrieving revision 1.44
> > diff -u -r1.44 http_config.c
> > --- http_config.c 2000/04/26 07:14:32 1.44
> > +++ http_config.c 2000/04/27 11:53:38
> > @@ -865,6 +865,7 @@
> >
> > if (cmd_name[0] == '<') {
> > if (cmd_name[1] != '/') {
> > + newdir->container = 1;
> > (*current) = ap_add_node(curr_parent, *current, newdir, 1);
> > }
> > else if (*curr_parent == NULL) {
> > @@ -976,6 +977,12 @@
> >
> > if (*conftree == NULL && current != NULL) {
> > *conftree = current;
> > + if (!curr_parent) {
> > + if (current->container) {
> > + curr_parent = current;
> > + current = NULL;
> > + }
> > + }
> > }
> > }
> >
> > Index: include/util_cfgtree.h
> > ===================================================================
> > RCS file: /cvs/apache/apache-2.0/src/include/util_cfgtree.h,v
> > retrieving revision 1.2
> > diff -u -r1.2 util_cfgtree.h
> > --- util_cfgtree.h 2000/04/24 12:00:43 1.2
> > +++ util_cfgtree.h 2000/04/27 11:53:38
> > @@ -65,6 +65,7 @@
> > void *data; /* directive's module can store add'l data here */
> >
> > /* ### these may go away in the future, but are needed for now */
> > + int container;
> > const char *filename;
> > int line_num;
> >
> >
> >
> > --
> > Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> > http://www.geocities.com/SiliconValley/Park/9289/
> > Born in Roswell... married an alien...
> >
>
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Greg Stein <gs...@lyra.org>.
On Thu, 27 Apr 2000 rbb@covalent.net wrote:
> The problem is that the tree is no longer being built correctly. This
It never was (when we hit two containers in a row). That bug has been
present since the first commit.
>...
> While we want it to look like:
>
> (ifmod node)
> |
> -> (startservers node) -> (/ifmod node)
>
> The /ifmod nodes may not be there anymore.
Correct, and yes: the /ifmod node is no longer present. It is unnecessary
since the tree itself implies the "close."
Cheers,
-g
>...
> > <IfModule prefork.c>
> > MinSpareServers 20
> > MaxSpareServers 40
> > </IfModule>
> > servername gobble
>...
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by rb...@covalent.net.
The problem is that the tree is no longer being built correctly. This
patch does not fix the underlying problem. I am looking into this right
now.
>From what I can see, the tree currently looks like:
(ifmod node) -> (StartServers node) -> (/ifmod node)
While we want it to look like:
(ifmod node)
|
-> (startservers node) -> (/ifmod node)
The /ifmod nodes may not be there anymore.
I'll post a patch when I fix it.
Ryan
On Thu, 27 Apr 2000, Jeff Trawick wrote:
> Here is a patch that works for a larger set of cfg files :)
>
> The problem (or one problem) seems to be that curr_parent/current
> don't get set up properly when the first element of the cfg file is a
> container.
>
> Maybe these changes will give you some hints. With these changes,
> this config file looks real pretty in ddd:
>
> <IfModule prefork.c>
> MinSpareServers 20
> MaxSpareServers 40
> </IfModule>
> servername gobble
>
> Without the changes, MinSpareServers is pointed to by the next pointer
> of "<IfModule prefork.c>" and "</IfModule>" is supposedly dangling.
>
> Index: main/http_config.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/main/http_config.c,v
> retrieving revision 1.44
> diff -u -r1.44 http_config.c
> --- http_config.c 2000/04/26 07:14:32 1.44
> +++ http_config.c 2000/04/27 11:53:38
> @@ -865,6 +865,7 @@
>
> if (cmd_name[0] == '<') {
> if (cmd_name[1] != '/') {
> + newdir->container = 1;
> (*current) = ap_add_node(curr_parent, *current, newdir, 1);
> }
> else if (*curr_parent == NULL) {
> @@ -976,6 +977,12 @@
>
> if (*conftree == NULL && current != NULL) {
> *conftree = current;
> + if (!curr_parent) {
> + if (current->container) {
> + curr_parent = current;
> + current = NULL;
> + }
> + }
> }
> }
>
> Index: include/util_cfgtree.h
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/include/util_cfgtree.h,v
> retrieving revision 1.2
> diff -u -r1.2 util_cfgtree.h
> --- util_cfgtree.h 2000/04/24 12:00:43 1.2
> +++ util_cfgtree.h 2000/04/27 11:53:38
> @@ -65,6 +65,7 @@
> void *data; /* directive's module can store add'l data here */
>
> /* ### these may go away in the future, but are needed for now */
> + int container;
> const char *filename;
> int line_num;
>
>
>
> --
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by rb...@covalent.net.
Well, with that news, the commit is on its way. :-)
Ryan
On Thu, 27 Apr 2000, Paul J. Reder wrote:
> Ryan and Jeff,
>
> I have verified that Ryan's patch fixes my scenario also.
>
> rbb@covalent.net wrote:
> >
> > > Good! (It works for me.)
> >
> > Great! I'll modify the comment, and commit later today, unless somebody
> > posts a file this doesn't work for. :-)
>
> --
>
> Paul J. Reder
>
> ---------------------------------------------------------------------
> Noise proves nothing. Often a hen who has merely laid an egg cackles
> as if she laid an asteroid.
> -- Mark Twain
> (An apt quote in this political season.)
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by "Paul J. Reder" <re...@raleigh.ibm.com>.
Ryan and Jeff,
I have verified that Ryan's patch fixes my scenario also.
rbb@covalent.net wrote:
>
> > Good! (It works for me.)
>
> Great! I'll modify the comment, and commit later today, unless somebody
> posts a file this doesn't work for. :-)
--
Paul J. Reder
---------------------------------------------------------------------
Noise proves nothing. Often a hen who has merely laid an egg cackles
as if she laid an asteroid.
-- Mark Twain
(An apt quote in this political season.)
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by rb...@covalent.net.
> Good! (It works for me.)
Great! I'll modify the comment, and commit later today, unless somebody
posts a file this doesn't work for. :-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Jeff Trawick <tr...@bellsouth.net>.
> I'm attaching another patch to fix this problem. Paul, please try out
> this patch and let me know if this solves the problem you are hitting. I
> like this patch more, because it fits into the current code a bit
> cleaner. All of the changes are in ap_add_node. This works for me for
> both the default config and the config file that Jeff posted
> yesterday. :-)
Good! (It works for me.)
(This is functionally equivalent to the additional change I posted a
minute or two ago, but is better since it uses the child parameter
instead of my unnecessary container field. I didn't realize that my
second change was the general solution for both broken config files I
posted earlier today.)
Note the tweak below to the comment... This tweak reflects that
problem 2 (in my post earlier today, where a container is the first
thing in a container) is fixed by that code as well. (Greg Ames and I
both started seeing problem 2 a few days ago but we were heads down on
something else and deferred it for a while.)
> Index: main/util_cfgtree.c
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/main/util_cfgtree.c,v
> retrieving revision 1.3
> diff -u -d -b -w -u -r1.3 util_cfgtree.c
> --- main/util_cfgtree.c 2000/04/22 22:54:30 1.3
> +++ main/util_cfgtree.c 2000/04/27 17:47:13
> @@ -69,6 +69,11 @@
> (*parent)->first_child = toadd;
> toadd->parent = *parent;
> }
> + if (child) {
> + /* First item in config file is a container */
---> /* First item in config file or container is a container */
> + *parent = toadd;
> + return NULL;
> + }
> return toadd;
> }
> current->next = toadd;
>
>
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
>
>
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by rb...@covalent.net.
I'm attaching another patch to fix this problem. Paul, please try out
this patch and let me know if this solves the problem you are hitting. I
like this patch more, because it fits into the current code a bit
cleaner. All of the changes are in ap_add_node. This works for me for
both the default config and the config file that Jeff posted
yesterday. :-)
Index: main/util_cfgtree.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/main/util_cfgtree.c,v
retrieving revision 1.3
diff -u -d -b -w -u -r1.3 util_cfgtree.c
--- main/util_cfgtree.c 2000/04/22 22:54:30 1.3
+++ main/util_cfgtree.c 2000/04/27 17:47:13
@@ -69,6 +69,11 @@
(*parent)->first_child = toadd;
toadd->parent = *parent;
}
+ if (child) {
+ /* First item in config file is a container */
+ *parent = toadd;
+ return NULL;
+ }
return toadd;
}
current->next = toadd;
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Jeff Trawick <tr...@bellsouth.net>.
Here is a patch that works for a larger set of cfg files :)
The problem (or one problem) seems to be that curr_parent/current
don't get set up properly when the first element of the cfg file is a
container.
Maybe these changes will give you some hints. With these changes,
this config file looks real pretty in ddd:
<IfModule prefork.c>
MinSpareServers 20
MaxSpareServers 40
</IfModule>
servername gobble
Without the changes, MinSpareServers is pointed to by the next pointer
of "<IfModule prefork.c>" and "</IfModule>" is supposedly dangling.
Index: main/http_config.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/main/http_config.c,v
retrieving revision 1.44
diff -u -r1.44 http_config.c
--- http_config.c 2000/04/26 07:14:32 1.44
+++ http_config.c 2000/04/27 11:53:38
@@ -865,6 +865,7 @@
if (cmd_name[0] == '<') {
if (cmd_name[1] != '/') {
+ newdir->container = 1;
(*current) = ap_add_node(curr_parent, *current, newdir, 1);
}
else if (*curr_parent == NULL) {
@@ -976,6 +977,12 @@
if (*conftree == NULL && current != NULL) {
*conftree = current;
+ if (!curr_parent) {
+ if (current->container) {
+ curr_parent = current;
+ current = NULL;
+ }
+ }
}
}
Index: include/util_cfgtree.h
===================================================================
RCS file: /cvs/apache/apache-2.0/src/include/util_cfgtree.h,v
retrieving revision 1.2
diff -u -r1.2 util_cfgtree.h
--- util_cfgtree.h 2000/04/24 12:00:43 1.2
+++ util_cfgtree.h 2000/04/27 11:53:38
@@ -65,6 +65,7 @@
void *data; /* directive's module can store add'l data here */
/* ### these may go away in the future, but are needed for now */
+ int container;
const char *filename;
int line_num;
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Greg Stein <gs...@lyra.org>.
On Wed, 26 Apr 2000 rbb@covalent.net wrote:
> I can get the error using the three line config you posted, but not using
> the default config. I'll look at your patch and try to figure out what
> the problem is today or EARLY tomorrow.
Yes, unfortunately, I was testing with the default config also.
Ryan's recent commit was the "proper" commit. If two (nested) containers
were found in sequence, we did not construct the tree properly. These tiny
configs expose the problem since the file itself is effectively a
container.
Note that Jeff's patch would not have been appropriate -- it would have
set a parent for the top-level directives. Those directives' parent
pointer should be NULL (no parent).
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by rb...@covalent.net.
I can get the error using the three line config you posted, but not using
the default config. I'll look at your patch and try to figure out what
the problem is today or EARLY tomorrow.
This is a priority for me, so unless somebody beats me to it, I'll do it
tomorrow morning. (Unfortunately, I have another priority that is taking
precedence) :-)
Ryan
On Wed, 26 Apr 2000, Jeff Trawick wrote:
> > I'm not hitting this bug, and I've been exercising the config stuff pretty
> > hard recently. Are you using the default config?
> >
> > Ryan
>
> I was using the default config, but I can even hit it with a
> three-line config file:
>
> [trawick@cs390-1 src]$ cat /dl/apache/apache-2.0/src/tmp.conf
> <IfModule prefork.c>
> StartServers 10
> </IfModule>
> [trawick@cs390-1 src]$ ./httpd -f /dl/apache/apache-2.0/src/tmp.conf
> Syntax error on line 3 of /dl/apache/apache-2.0/src/tmp.conf:
> </IfModule> without matching <IfModule> section
> [trawick@cs390-1 src]$
>
> This copy of apache-2.0 is supposedly in sync with the anoncvs server.
>
> I'll play with it a bit more.
> --
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] config tree bug (is anybody hitting one)???
Posted by Jeff Trawick <tr...@bellsouth.net>.
> I'm not hitting this bug, and I've been exercising the config stuff pretty
> hard recently. Are you using the default config?
>
> Ryan
I was using the default config, but I can even hit it with a
three-line config file:
[trawick@cs390-1 src]$ cat /dl/apache/apache-2.0/src/tmp.conf
<IfModule prefork.c>
StartServers 10
</IfModule>
[trawick@cs390-1 src]$ ./httpd -f /dl/apache/apache-2.0/src/tmp.conf
Syntax error on line 3 of /dl/apache/apache-2.0/src/tmp.conf:
</IfModule> without matching <IfModule> section
[trawick@cs390-1 src]$
This copy of apache-2.0 is supposedly in sync with the anoncvs server.
I'll play with it a bit more.
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...