You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Ames <gr...@remulak.net> on 2001/08/11 21:12:29 UTC
seg fault in 2.0.23 :-(
I put a new 2.0.23 build up very briefly on daedalus, but had to take it
down shortly after due to taking a bunch of seg faults. The dump is
/usr/local/apache2_0_23/corefiles/httpd.core.1 if anybody wants to have
a look. I'm pretty burned out at the moment.
find_ct() at line 920 in mod_mime is calling apr_strcat with a bad
charset ptr. It's an autoindex request for
httpd.apache.org/dist/httpd/old/, running a subrequest on
apache_1.3.1.tar.Z to figure out which icon to put on the line, I
imagine. find_ct() recently had optimizations done.
(gdb) list
915 NULL);
916 }
917 pp = pp->next;
918 }
919 if (charset && !override) {
920 r->content_type = apr_pstrcat(r->pool,
r->content_type,
921 "; charset=",
charset,
922 NULL);
923 }
924 }
(gdb) p charset
$4 = 0x2 <Address 0x2 out of bounds>
(gdb) bt
#0 0x80818a6 in apr_pstrcat (a=0x81ea00c) at apr_strings.c:122
#1 0x281b1274 in find_ct (r=0x81ea03c) at mod_mime.c:920
#2 0x8070bd4 in ap_run_type_checker (r=0x81ea03c) at request.c:112
#3 0x8072054 in ap_sub_req_lookup_dirent (dirent=0xbfbfd7c8,
r=0x815003c,
next_filter=0x0) at request.c:1458
#4 0x281cd1e0 in make_autoindex_entry (dirent=0xbfbfd7c8,
autoindex_opts=8196, d=0x81e355c, r=0x815003c, keyid=78 'N',
direction=65 'A', pattern=0x0) at mod_autoindex.c:1260
#5 0x281cea6b in index_directory (r=0x815003c,
autoindex_conf=0x81e355c)
at mod_autoindex.c:1951
#6 0x281cec22 in handle_autoindex (r=0x815003c) at mod_autoindex.c:2013
#7 0x8062520 in ap_run_handler (r=0x815003c) at config.c:185
#8 0x806299b in ap_invoke_handler (r=0x815003c) at config.c:344
#9 0x806006c in process_request_internal (r=0x815003c) at
http_request.c:378#10 0x806014a in ap_process_request (r=0x815003c) at
http_request.c:444
#11 0x805c31a in ap_process_http_connection (c=0x814a114) at
http_core.c:287
#12 0x8069504 in ap_run_process_connection (c=0x814a114) at
connection.c:82
#13 0x8069688 in ap_process_connection (c=0x814a114) at connection.c:219
#14 0x8061226 in child_main (child_num_arg=47) at prefork.c:814
#15 0x806136e in make_child (s=0x80c2554, slot=47) at prefork.c:901
#16 0x80613ce in startup_children (number_to_start=50) at prefork.c:924
#17 0x8061727 in ap_mpm_run (_pconf=0x80c200c, plog=0x80f200c,
s=0x80c2554)
at prefork.c:1139
Greg
Re: seg fault in 2.0.23 :-(
Posted by Ryan Bloom <rb...@covalent.net>.
On Tuesday 14 August 2001 05:00, Bill Stoddard wrote:
> On your local machine, replicate much of the httpd.apache.orh/dist
> directory, including the .htaccess file. Enable overrides in that
> directory.
>
> Then add this to your conf file:
>
> <Files *.asc>
> RemoveEncoding gz tgz Z
> </Files>
>
> See if that let's you hit the seg fault.
Nope. If I have time tonight (unlikely), I will try to reproduce this again. Right
now, I need to head to the office, and I have other bugs awaiting me there.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: seg fault in 2.0.23 :-(
Posted by Bill Stoddard <bi...@wstoddard.com>.
On your local machine, replicate much of the httpd.apache.orh/dist directory, including the
.htaccess file. Enable overrides in that directory.
Then add this to your conf file:
<Files *.asc>
RemoveEncoding gz tgz Z
</Files>
See if that let's you hit the seg fault.
Bill
>
> > I think the problem might be in this block:
> >
> > static void *merge_mime_dir_configs(apr_pool_t *p, void *basev, void *addv)
> > {
> > mime_dir_config *base = (mime_dir_config *) basev;
> > mime_dir_config *add = (mime_dir_config *) addv;
> > mime_dir_config *new = apr_palloc(p, sizeof(mime_dir_config));
> >
> > int i;
> > attrib_info *suffix;
> >
> > if (base->extension_mappings && add->extension_mappings) {
> > if (base->copy_mappings)
> > new->extension_mappings = base->extension_mappings;
> > else {
> > new->extension_mappings = apr_hash_make(p);
> > overlay_extension_mappings(p, base->extension_mappings,
> > new->extension_mappings);
> > }
> > overlay_extension_mappings(p, add->extension_mappings,
> > new->extension_mappings);
> >
> > If base->copy_mappings is true upon entry into the function,
> > then the call to overlay_extension_mappings at the end will
> > use p to allocate data for insertion into base->extension_mappings.
> > If base->extension_mappings has been allocated from a pool with
> > a longer lifetime than p, then we'll eventually segv.
>
> If this is happening, then we have much bigger problems. All of the code
> that is involved with these hash tables is run at server startup. It is all run
> using pconf. The only time this might not be true, is if we have a .htaccess
> file that is changing the config. That might be able to cause this problem, but
> I have been unable to reproduce this. I have tried with a .htaccess, but it is
> also possible that another part of my config is not setup properly.
>
> Ryan
>
> ______________________________________________________________
> Ryan Bloom rbb@apache.org
> Covalent Technologies rbb@covalent.net
> --------------------------------------------------------------
>
Re: seg fault in 2.0.23 :-(
Posted by Ryan Bloom <rb...@covalent.net>.
> I think the problem might be in this block:
>
> static void *merge_mime_dir_configs(apr_pool_t *p, void *basev, void *addv)
> {
> mime_dir_config *base = (mime_dir_config *) basev;
> mime_dir_config *add = (mime_dir_config *) addv;
> mime_dir_config *new = apr_palloc(p, sizeof(mime_dir_config));
>
> int i;
> attrib_info *suffix;
>
> if (base->extension_mappings && add->extension_mappings) {
> if (base->copy_mappings)
> new->extension_mappings = base->extension_mappings;
> else {
> new->extension_mappings = apr_hash_make(p);
> overlay_extension_mappings(p, base->extension_mappings,
> new->extension_mappings);
> }
> overlay_extension_mappings(p, add->extension_mappings,
> new->extension_mappings);
>
> If base->copy_mappings is true upon entry into the function,
> then the call to overlay_extension_mappings at the end will
> use p to allocate data for insertion into base->extension_mappings.
> If base->extension_mappings has been allocated from a pool with
> a longer lifetime than p, then we'll eventually segv.
If this is happening, then we have much bigger problems. All of the code
that is involved with these hash tables is run at server startup. It is all run
using pconf. The only time this might not be true, is if we have a .htaccess
file that is changing the config. That might be able to cause this problem, but
I have been unable to reproduce this. I have tried with a .htaccess, but it is
also possible that another part of my config is not setup properly.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: seg fault in 2.0.23 :-(
Posted by Brian Pane <bp...@pacbell.net>.
Bill Stoddard wrote:
>>On Mon, Aug 13, 2001 at 08:26:12PM -0400, Bill Stoddard wrote:
>>
>>>I was just able to recreate this on my Windows machine. Will post an analysys (and
>>>
>perhaps
>
>>>a fix) later on.
>>>
>>I have a potential fix but no ability to recreate the fault. ;-)
>>Given the butt-ugly nature of the current code, I'll commit what I have and
>>you can take it from there.
>>
>>....Roy
>>
>
>Still broken after your commit.
>
>The problem is that we are creating an extension_info out of a subrequest pool and
>sticking it into a longer lived hash table (allocated out of the request pool I'm
>guessing). When we pull the extension_info out of the hash table later and try to use it,
>we seg fault because the storage has been reused for something else. No thoughts on a fix.
>
I think the problem might be in this block:
static void *merge_mime_dir_configs(apr_pool_t *p, void *basev, void *addv)
{
mime_dir_config *base = (mime_dir_config *) basev;
mime_dir_config *add = (mime_dir_config *) addv;
mime_dir_config *new = apr_palloc(p, sizeof(mime_dir_config));
int i;
attrib_info *suffix;
if (base->extension_mappings && add->extension_mappings) {
if (base->copy_mappings)
new->extension_mappings = base->extension_mappings;
else {
new->extension_mappings = apr_hash_make(p);
overlay_extension_mappings(p, base->extension_mappings,
new->extension_mappings);
}
overlay_extension_mappings(p, add->extension_mappings,
new->extension_mappings);
If base->copy_mappings is true upon entry into the function,
then the call to overlay_extension_mappings at the end will
use p to allocate data for insertion into base->extension_mappings.
If base->extension_mappings has been allocated from a pool with
a longer lifetime than p, then we'll eventually segv.
--Brian
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in
2.0.23 :-)
Posted by Greg Ames <gr...@remulak.net>.
Bill Stoddard wrote:
>
> I prefer this patch which creates a completely new, fully merged, mime_dir_config on each
> call to merge_mime_dir_configs. It is a bit more expensive but should it still work better
> than the table code (maybe. I'd like to see it profiled :-). And this code is a hell of a
> lot easier to understand :-)
OK, I like simplicity too. But unfortunately I got:
patching file modules/http/mod_mime.c
Hunk #4 FAILED at 212.
Hunk #5 FAILED at 231.
Hunk #6 FAILED at 245.
Hunk #7 FAILED at 259.
Hunk #9 succeeded at 769 with fuzz 1.
4 out of 9 hunks FAILED -- saving rejects to file
modules/http/mod_mime.c.rej
...when I tried it. Was this patch against the current HEAD?
Greg
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
The base config is created in cfg->pool (a child of process->pool.)
If we are supposed to merge into the r->pool, we need a copy. That copy gets
tagged with the r->pool
If we are then supposed to merge using the sub_req->pool, we need another copy.
That copy is tagged with that sub_req->pool.
If we start caching configs, each cached <Directory /> config needs it's own pool
(so it can be destroyed when the cache stales.) That cache->pool is different
than the base cfg->pool, so we must copy. When we take the cached config into the
request, cache->pool isn't r->pool, so again we copy.
Other than that, we can keep merging without copies. Otherwise we drop the hashing,
because it is a cpu buster.
Bill
----- Original Message -----
From: "Bill Stoddard" <bi...@wstoddard.com>
To: <ne...@apache.org>
Sent: Tuesday, August 14, 2001 4:21 PM
Subject: Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
>
> > -1 (veto) to both suggestions.
> >
> > This is getting obscene. We've lost all value of tables, since they can't be memcpy'ed,
> > they have to be reconstructed. This will be much slower than the original array code.
> >
> > Committers, please review dir_create() and dir_merge() patches more carefully. There
> > is a long tradition of configuration bugs due to these assumptions. The patches that
> > added copy/copy_mappings was an attempt to salvage this.
> >
> > I'll demonstrate a solution, using pool scopes, that adds safety and speed. Give me a
> day.
>
> What are pool scopes?
>
> Bill
>
>
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by Bill Stoddard <bi...@wstoddard.com>.
> -1 (veto) to both suggestions.
>
> This is getting obscene. We've lost all value of tables, since they can't be memcpy'ed,
> they have to be reconstructed. This will be much slower than the original array code.
>
> Committers, please review dir_create() and dir_merge() patches more carefully. There
> is a long tradition of configuration bugs due to these assumptions. The patches that
> added copy/copy_mappings was an attempt to salvage this.
>
> I'll demonstrate a solution, using pool scopes, that adds safety and speed. Give me a
day.
What are pool scopes?
Bill
Re: [PATCH] solves the mod_mime seg fault without loosing the hash?
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Brian Pane" <bp...@pacbell.net>
Sent: Wednesday, August 15, 2001 12:05 AM
> William A. Rowe, Jr. wrote:
> [...]
>
> >+static void remove_items(apr_pool_t *p, apr_array_header_t *remove,
> >+ apr_hash_t *mappings, apr_size_t member)
> >+{
> >+ attrib_info *suffix = (attrib_info *) remove->elts;
> >+ int i;
> >+ for (i = 0; i < remove->nelts; i++) {
> >+ extension_info *exinfo =
> >+ (extension_info*)apr_hash_get(mappings,
> >+ suffix[i].name,
> >+ APR_HASH_KEY_STRING);
> >+ /* If we find the entry, the given member isn't NULL,
> >+ * and this exinfo wasn't allocated from the current pool,
> >+ * then we must copy before we remove the given member.
> >+ */
> >
> Why is it necessary to make a copy before overwriting the
> field in exinfo with NULL?
The hash members are _not_ copied by the apr_hash functions! This is why the
original patch had broken Apache.
The hash member has 5 pointers. This function nullifies one of them. If you don't
copy first, you are actually wiping out the registered value of the original config.
Worse, (in the overlay case) if you replace one of those pointers due to an .htaccess
override, you have just pointed into the r-> pool, so once this request is released,
the next request would segfault the server.
> >
> >+ if (!base->extension_mappings && !add->extension_mappings)
> >+ new->extension_mappings = NULL;
> >+ else if (base->extension_mappings
> >+ && ((base->pool == p) || !(add->extension_mappings
> >+ || add->handlers_remove
> >+ || add->types_remove
> >+ || add->encodings_remove)))
> >+ new->extension_mappings = base->extension_mappings;
> >+ else {
> >+ new->extension_mappings = apr_hash_make(p);
> >+ if (base->extension_mappings)
> > overlay_extension_mappings(p, base->extension_mappings,
> > new->extension_mappings);
> >+ }
> >
> One minor problem here (and I can't remember if it will be
> a problem in practice): this logic can call overlay_extension_mappings
> with one of its latter two arguments equal to NULL, and
> overlay_extension_mappings
> will dereference NULL without checking.
That's why we test first. new->extension_mappings is guarenteed to be non-null.
Bill
Re: [PATCH] solves the mod_mime seg fault without loosing the hash?
Posted by Brian Pane <bp...@pacbell.net>.
William A. Rowe, Jr. wrote:
[...]
>+static void remove_items(apr_pool_t *p, apr_array_header_t *remove,
>+ apr_hash_t *mappings, apr_size_t member)
>+{
>+ attrib_info *suffix = (attrib_info *) remove->elts;
>+ int i;
>+ for (i = 0; i < remove->nelts; i++) {
>+ extension_info *exinfo =
>+ (extension_info*)apr_hash_get(mappings,
>+ suffix[i].name,
>+ APR_HASH_KEY_STRING);
>+ /* If we find the entry, the given member isn't NULL,
>+ * and this exinfo wasn't allocated from the current pool,
>+ * then we must copy before we remove the given member.
>+ */
>
Why is it necessary to make a copy before overwriting the
field in exinfo with NULL?
[...]
>
>+ if (!base->extension_mappings && !add->extension_mappings)
>+ new->extension_mappings = NULL;
>+ else if (base->extension_mappings
>+ && ((base->pool == p) || !(add->extension_mappings
>+ || add->handlers_remove
>+ || add->types_remove
>+ || add->encodings_remove)))
>+ new->extension_mappings = base->extension_mappings;
>+ else {
>+ new->extension_mappings = apr_hash_make(p);
>+ if (base->extension_mappings)
> overlay_extension_mappings(p, base->extension_mappings,
> new->extension_mappings);
>+ }
>
One minor problem here (and I can't remember if it will be
a problem in practice): this logic can call overlay_extension_mappings
with one of its latter two arguments equal to NULL, and
overlay_extension_mappings
will dereference NULL without checking.
--Brian
Re: [PATCH] solves the mod_mime seg fault without loosing the hash?
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "William A. Rowe, Jr." <wr...@rowe-clan.net>
Sent: Tuesday, August 14, 2001 9:51 PM
> > OK. More complex, and not 'tweaked' for legibility. Rather than make you all wait,
> > I'm posting as-is, and won't commit yet, since we have some contention about the 'right'
> > solution.
>
> Also introduces or reveals the segfault for me... I'm reviewing now, you might want to
> wait just a bit, I'll post a revision in an hour or so.
Sorry. As usual (when you hide things in nasty casts) it pays to test.
Attached is the proper patch. Please, test this against the segfault cases.
Bill
Re: [PATCH] solves the mod_mime seg fault without loosing the hash?
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "William A. Rowe, Jr." <wr...@rowe-clan.net>
Sent: Tuesday, August 14, 2001 9:39 PM
> OK. More complex, and not 'tweaked' for legibility. Rather than make you all wait,
> I'm posting as-is, and won't commit yet, since we have some contention about the 'right'
> solution.
Also introduces or reveals the segfault for me... I'm reviewing now, you might want to
wait just a bit, I'll post a revision in an hour or so.
Bill
[PATCH] solves the mod_mime seg fault without loosing the hash?
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Bill Stoddard" <bi...@wstoddard.com>
Sent: Tuesday, August 14, 2001 4:26 PM
> > This is getting obscene. We've lost all value of tables, since they can't be memcpy'ed,
> > they have to be reconstructed. This will be much slower than the original array code.
The option, Bill, to eliminate the hash entirely, is still in the realm of consideration.
But that patch can't undo Roy's and my bug fixes (that weren't related to the hash.)
> > I'll demonstrate a solution, using pool scopes, that adds safety and speed. Give me a day.
>
> Hit send before I was finished. If pool scopes significantly increases the complexity of
> this code, I will likely veto it. So you might want to share what you are planning on
> doing before you invest much time in it. I learned a lesson working on this code... the
> dir_create() and dir_merge() code needs to be as straight forward and easy to understand
> as possible to keep bugs out. Gaining 5% performance at the expense of a 100% increase in
> code complexity is not an acceptable trade off. The copy_mappings, et. al. was more like a
> 1000% increase in complexity.
OK. More complex, and not 'tweaked' for legibility. Rather than make you all wait,
I'm posting as-is, and won't commit yet, since we have some contention about the 'right'
solution.
The list of 'optimizable cases' shrinks in this patch. ->copy/copy_mappings is gone.
We only care that the cfg->pool and entry->pool are in-scope. If they aren't, we copy.
We've got to copy when we merge or remove. OH ... this patch defers the forced
'hash_make' until we actually add an entry. You may want to ignore that change (set
new->extension_mappings = apr_hash_make(p); in line 152) to continue to reproduce the
segfault reliably.
remove-items is kluged to eliminate a ton of lines (three reps). Less prone to error,
I hope. The big headache is the 'member' index into the exinfo structure. That 'offset'
code gets ugly, perhaps someone has a better/cleaner solution (using a macro or some such.)
Anyways, I've wrapped my brain around this another three hours. Try this patch if you can
reproduce the segfault (I can't, but couldn't before) and report back.
Bill
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by Bill Stoddard <bi...@wstoddard.com>.
> -1 (veto) to both suggestions.
>
> This is getting obscene. We've lost all value of tables, since they can't be memcpy'ed,
> they have to be reconstructed. This will be much slower than the original array code.
>
> Committers, please review dir_create() and dir_merge() patches more carefully. There
> is a long tradition of configuration bugs due to these assumptions. The patches that
> added copy/copy_mappings was an attempt to salvage this.
>
> I'll demonstrate a solution, using pool scopes, that adds safety and speed. Give me a
day.
>
Hit send before I was finished. If pool scopes significantly increases the complexity of
this code, I will likely veto it. So you might want to share what you are planning on
doing before you invest much time in it. I learned a lesson working on this code... the
dir_create() and dir_merge() code needs to be as straight forward and easy to understand
as possible to keep bugs out. Gaining 5% performance at the expense of a 100% increase in
code complexity is not an acceptable trade off. The copy_mappings, et. al. was more like a
1000% increase in complexity.
Bill
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
The sensible thing would be to stop putting so many implementation
assumptions throughout the code. If we separated the act of checking
the value from the nature of the config data structure, then we could
have look-up routines that acted on a list of layered configs instead
of copying each layer onto another onto another for every request.
That would speed up the high-performance case of no-overrides and
eliminate mixing of pools.
In the mean time, though, let's just fix the seg fault.
....Roy
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Brian Pane" <bp...@pacbell.net>
Sent: Tuesday, August 14, 2001 5:37 PM
> Are you still considering optimizing directory_walk so that it
> doesn't have to do all the dir_merge work on each request (either
> through a caching solution or through some variant of the pre-merge
> patch that I posted a while back)?
No, I think we _do_ want some pre-merge cache. That doesn't oblivate the
need to optimize one of the most top-heavy aspects of Apache modules.
Unless you are hiding all your Alias'es at a root directory, there can
be alot of this going on.
> If we didn't have to call merge_mime_dir_config with such alarming
> frequency, it might not be necessary to optimize this function (or
> dir_merge functions in general).
I still think optimizing dir merges is a defacto requirement. We just
have to do it gracefully (by pool scope, or whatever.)
Bill
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in
2.0.23 :-)
Posted by Brian Pane <bp...@pacbell.net>.
William A. Rowe, Jr. wrote:
>-1 (veto) to both suggestions.
>
>This is getting obscene. We've lost all value of tables, since they can't be memcpy'ed,
>they have to be reconstructed. This will be much slower than the original array code.
>
>Committers, please review dir_create() and dir_merge() patches more carefully. There
>is a long tradition of configuration bugs due to these assumptions. The patches that
>added copy/copy_mappings was an attempt to salvage this.
>
>I'll demonstrate a solution, using pool scopes, that adds safety and speed. Give me a day.
>
Are you still considering optimizing directory_walk so that it
doesn't have to do all the dir_merge work on each request (either
through a caching solution or through some variant of the pre-merge
patch that I posted a while back)?
If we didn't have to call merge_mime_dir_config with such alarming
frequency, it might not be necessary to optimize this function (or
dir_merge functions in general).
--Brian
Re: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
-1 (veto) to both suggestions.
This is getting obscene. We've lost all value of tables, since they can't be memcpy'ed,
they have to be reconstructed. This will be much slower than the original array code.
Committers, please review dir_create() and dir_merge() patches more carefully. There
is a long tradition of configuration bugs due to these assumptions. The patches that
added copy/copy_mappings was an attempt to salvage this.
I'll demonstrate a solution, using pool scopes, that adds safety and speed. Give me a day.
Bill
----- Original Message -----
From: "Bill Stoddard" <bi...@wstoddard.com>
To: <ne...@apache.org>
Sent: Tuesday, August 14, 2001 3:57 PM
Subject: [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
> I prefer this patch which creates a completely new, fully merged, mime_dir_config on each
> call to merge_mime_dir_configs. It is a bit more expensive but should it still work better
> than the table code (maybe. I'd like to see it profiled :-). And this code is a hell of a
> lot easier to understand :-)
>
> Bill
>
> Index: mod_mime.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
> retrieving revision 1.50
> diff -u -r1.50 mod_mime.c
> --- mod_mime.c 2001/08/14 02:35:55 1.50
> +++ mod_mime.c 2001/08/14 20:46:56
> @@ -105,14 +105,11 @@
> char *language_type; /* Added with AddLanguage... */
> char *handler; /* Added with AddHandler... */
> char *charset_type; /* Added with AddCharset... */
> - int copy; /* To avoid dupping in the merge */
> } extension_info;
>
> typedef struct {
> apr_hash_t *extension_mappings; /* Map from extension name to
> * extension_info structure */
> - int copy_mappings; /* To avoid dupping in the merge */
> -
> apr_array_header_t *handlers_remove; /* List of handlers to remove */
> apr_array_header_t *types_remove; /* List of MIME types to remove */
> apr_array_header_t *encodings_remove; /* List of encodings to remove */
> @@ -150,7 +147,6 @@
> (mime_dir_config *) apr_palloc(p, sizeof(mime_dir_config));
>
> new->extension_mappings = apr_hash_make(p);
> - new->copy_mappings = 0;
>
> new->handlers_remove = NULL;
> new->types_remove = NULL;
> @@ -180,13 +176,10 @@
> base_info = (extension_info*)apr_hash_get(base, key, klen);
>
> if (base_info) {
> - if (!base_info->copy) {
> - extension_info *copyinfo = base_info;
> - base_info = (extension_info*)apr_palloc(p, sizeof(*base_info));
> - apr_hash_set(base, key, klen, base_info);
> - memcpy(base_info, copyinfo, sizeof(*base_info));
> - base_info->copy = 1;
> - }
> + extension_info *copyinfo = base_info;
> + base_info = (extension_info*)apr_palloc(p, sizeof(*base_info));
> + apr_hash_set(base, key, klen, base_info);
> + memcpy(base_info, copyinfo, sizeof(*base_info));
>
> if (overlay_info->forced_type) {
> base_info->forced_type = overlay_info->forced_type;
> @@ -219,36 +212,15 @@
> int i;
> attrib_info *suffix;
>
> - if (base->extension_mappings && add->extension_mappings) {
> - if (base->copy_mappings)
> - new->extension_mappings = base->extension_mappings;
> - else {
> - new->extension_mappings = apr_hash_make(p);
> - overlay_extension_mappings(p, base->extension_mappings,
> - new->extension_mappings);
> - }
> - overlay_extension_mappings(p, add->extension_mappings,
> - new->extension_mappings);
> - new->copy_mappings = 1;
> + new->extension_mappings = apr_hash_make(p);
> +
> + if (base->extension_mappings != NULL) {
> + overlay_extension_mappings(p, base->extension_mappings,
> + new->extension_mappings);
> }
> - else {
> - if (base->extension_mappings == NULL) {
> - new->extension_mappings = add->extension_mappings;
> - new->copy_mappings = add->copy_mappings;
> - }
> - else if (add->extension_mappings == NULL) {
> - new->extension_mappings = base->extension_mappings;
> - new->copy_mappings = base->copy_mappings;
> - }
> - if (!new->copy_mappings && (add->handlers_remove
> - || add->types_remove
> - || add->encodings_remove)) {
> - apr_hash_t *copyhash = new->extension_mappings;
> - new->extension_mappings = apr_hash_make(p);
> - /* ### as slow as can be... just use an apr_hash_dup! */
> - overlay_extension_mappings(p, copyhash, new->extension_mappings);
> - new->copy_mappings = 1;
> - }
> + if (add->extension_mappings != NULL) {
> + overlay_extension_mappings(p, add->extension_mappings,
> + new->extension_mappings);
> }
>
> if (add->handlers_remove) {
> @@ -259,14 +231,6 @@
> suffix[i].name,
> APR_HASH_KEY_STRING);
> if (exinfo) {
> - if (!exinfo->copy) {
> - extension_info *copyinfo = exinfo;
> - exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
> - apr_hash_set(new->extension_mappings, suffix[i].name,
> - APR_HASH_KEY_STRING, exinfo);
> - memcpy(exinfo, copyinfo, sizeof(*exinfo));
> - exinfo->copy = 1;
> - }
> exinfo->handler = NULL;
> }
> }
> @@ -281,14 +245,6 @@
> suffix[i].name,
> APR_HASH_KEY_STRING);
> if (exinfo) {
> - if (!exinfo->copy) {
> - extension_info *copyinfo = exinfo;
> - exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
> - apr_hash_set(new->extension_mappings, suffix[i].name,
> - APR_HASH_KEY_STRING, exinfo);
> - memcpy(exinfo, copyinfo, sizeof(*exinfo));
> - exinfo->copy = 1;
> - }
> exinfo->forced_type = NULL;
> }
> }
> @@ -303,14 +259,6 @@
> suffix[i].name,
> APR_HASH_KEY_STRING);
> if (exinfo) {
> - if (!exinfo->copy) {
> - extension_info *copyinfo = exinfo;
> - exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
> - apr_hash_set(new->extension_mappings, suffix[i].name,
> - APR_HASH_KEY_STRING, exinfo);
> - memcpy(exinfo, copyinfo, sizeof(*exinfo));
> - exinfo->copy = 1;
> - }
> exinfo->encoding_type = NULL;
> }
> }
> @@ -792,6 +740,7 @@
> char *ext;
> const char *fn, *type, *charset = NULL;
> int found_metadata = 0;
> + extension_info *exinfo = NULL;
>
> if (r->finfo.filetype == APR_DIR) {
> r->content_type = DIR_MAGIC_TYPE;
> @@ -820,7 +769,7 @@
> /* Parse filename extensions which can be in any order
> */
> while (*fn && (ext = ap_getword(r->pool, &fn, '.'))) {
> - extension_info *exinfo;
> +
> int found;
>
> if (*ext == '\0') /* ignore empty extensions "bad..html" */
>
>
> ----- Original Message -----
> From: "Greg Ames" <gr...@remulak.net>
> To: <ne...@apache.org>
> Sent: Tuesday, August 14, 2001 3:50 PM
> Subject: [PATCH] for mod_mime seg fault in 2.0.23 :-)
>
>
> > Bill Stoddard wrote:
> > >
> >
> > > The problem is that we are creating an extension_info out of a subrequest pool and
> > > sticking it into a longer lived hash table (allocated out of the request pool I'm
> > > guessing). When we pull the extension_info out of the hash table later and try to use
> it,
> > > we seg fault because the storage has been reused for something else. No thoughts on a
> fix.
> >
> > This fixes the seg fault on my Linux box:
> >
> > Index: modules/http/mod_mime.c
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
> > retrieving revision 1.50
> > diff -u -d -b -r1.50 mod_mime.c
> > --- modules/http/mod_mime.c 2001/08/14 02:35:55 1.50
> > +++ modules/http/mod_mime.c 2001/08/14 19:39:45
> > @@ -220,13 +220,10 @@
> > attrib_info *suffix;
> >
> > if (base->extension_mappings && add->extension_mappings) {
> > - if (base->copy_mappings)
> > - new->extension_mappings = base->extension_mappings;
> > - else {
> > new->extension_mappings = apr_hash_make(p);
> > overlay_extension_mappings(p, base->extension_mappings,
> > new->extension_mappings);
> > - }
> > +
> > overlay_extension_mappings(p, add->extension_mappings,
> > new->extension_mappings);
> >
> > We can't allow overlay_extension_mappings to store things into the
> > extension_mappings hash table which have a shorter lifetime than the
> > hash table itself. Always using a new extension_mappings prevents this
> > from happening.
> >
> > I'll commit this in a little while unless someone objects soon.
> >
> > Greg
> > Greg
> >
>
>
[PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by Bill Stoddard <bi...@wstoddard.com>.
I prefer this patch which creates a completely new, fully merged, mime_dir_config on each
call to merge_mime_dir_configs. It is a bit more expensive but should it still work better
than the table code (maybe. I'd like to see it profiled :-). And this code is a hell of a
lot easier to understand :-)
Bill
Index: mod_mime.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
retrieving revision 1.50
diff -u -r1.50 mod_mime.c
--- mod_mime.c 2001/08/14 02:35:55 1.50
+++ mod_mime.c 2001/08/14 20:46:56
@@ -105,14 +105,11 @@
char *language_type; /* Added with AddLanguage... */
char *handler; /* Added with AddHandler... */
char *charset_type; /* Added with AddCharset... */
- int copy; /* To avoid dupping in the merge */
} extension_info;
typedef struct {
apr_hash_t *extension_mappings; /* Map from extension name to
* extension_info structure */
- int copy_mappings; /* To avoid dupping in the merge */
-
apr_array_header_t *handlers_remove; /* List of handlers to remove */
apr_array_header_t *types_remove; /* List of MIME types to remove */
apr_array_header_t *encodings_remove; /* List of encodings to remove */
@@ -150,7 +147,6 @@
(mime_dir_config *) apr_palloc(p, sizeof(mime_dir_config));
new->extension_mappings = apr_hash_make(p);
- new->copy_mappings = 0;
new->handlers_remove = NULL;
new->types_remove = NULL;
@@ -180,13 +176,10 @@
base_info = (extension_info*)apr_hash_get(base, key, klen);
if (base_info) {
- if (!base_info->copy) {
- extension_info *copyinfo = base_info;
- base_info = (extension_info*)apr_palloc(p, sizeof(*base_info));
- apr_hash_set(base, key, klen, base_info);
- memcpy(base_info, copyinfo, sizeof(*base_info));
- base_info->copy = 1;
- }
+ extension_info *copyinfo = base_info;
+ base_info = (extension_info*)apr_palloc(p, sizeof(*base_info));
+ apr_hash_set(base, key, klen, base_info);
+ memcpy(base_info, copyinfo, sizeof(*base_info));
if (overlay_info->forced_type) {
base_info->forced_type = overlay_info->forced_type;
@@ -219,36 +212,15 @@
int i;
attrib_info *suffix;
- if (base->extension_mappings && add->extension_mappings) {
- if (base->copy_mappings)
- new->extension_mappings = base->extension_mappings;
- else {
- new->extension_mappings = apr_hash_make(p);
- overlay_extension_mappings(p, base->extension_mappings,
- new->extension_mappings);
- }
- overlay_extension_mappings(p, add->extension_mappings,
- new->extension_mappings);
- new->copy_mappings = 1;
+ new->extension_mappings = apr_hash_make(p);
+
+ if (base->extension_mappings != NULL) {
+ overlay_extension_mappings(p, base->extension_mappings,
+ new->extension_mappings);
}
- else {
- if (base->extension_mappings == NULL) {
- new->extension_mappings = add->extension_mappings;
- new->copy_mappings = add->copy_mappings;
- }
- else if (add->extension_mappings == NULL) {
- new->extension_mappings = base->extension_mappings;
- new->copy_mappings = base->copy_mappings;
- }
- if (!new->copy_mappings && (add->handlers_remove
- || add->types_remove
- || add->encodings_remove)) {
- apr_hash_t *copyhash = new->extension_mappings;
- new->extension_mappings = apr_hash_make(p);
- /* ### as slow as can be... just use an apr_hash_dup! */
- overlay_extension_mappings(p, copyhash, new->extension_mappings);
- new->copy_mappings = 1;
- }
+ if (add->extension_mappings != NULL) {
+ overlay_extension_mappings(p, add->extension_mappings,
+ new->extension_mappings);
}
if (add->handlers_remove) {
@@ -259,14 +231,6 @@
suffix[i].name,
APR_HASH_KEY_STRING);
if (exinfo) {
- if (!exinfo->copy) {
- extension_info *copyinfo = exinfo;
- exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
- apr_hash_set(new->extension_mappings, suffix[i].name,
- APR_HASH_KEY_STRING, exinfo);
- memcpy(exinfo, copyinfo, sizeof(*exinfo));
- exinfo->copy = 1;
- }
exinfo->handler = NULL;
}
}
@@ -281,14 +245,6 @@
suffix[i].name,
APR_HASH_KEY_STRING);
if (exinfo) {
- if (!exinfo->copy) {
- extension_info *copyinfo = exinfo;
- exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
- apr_hash_set(new->extension_mappings, suffix[i].name,
- APR_HASH_KEY_STRING, exinfo);
- memcpy(exinfo, copyinfo, sizeof(*exinfo));
- exinfo->copy = 1;
- }
exinfo->forced_type = NULL;
}
}
@@ -303,14 +259,6 @@
suffix[i].name,
APR_HASH_KEY_STRING);
if (exinfo) {
- if (!exinfo->copy) {
- extension_info *copyinfo = exinfo;
- exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
- apr_hash_set(new->extension_mappings, suffix[i].name,
- APR_HASH_KEY_STRING, exinfo);
- memcpy(exinfo, copyinfo, sizeof(*exinfo));
- exinfo->copy = 1;
- }
exinfo->encoding_type = NULL;
}
}
@@ -792,6 +740,7 @@
char *ext;
const char *fn, *type, *charset = NULL;
int found_metadata = 0;
+ extension_info *exinfo = NULL;
if (r->finfo.filetype == APR_DIR) {
r->content_type = DIR_MAGIC_TYPE;
@@ -820,7 +769,7 @@
/* Parse filename extensions which can be in any order
*/
while (*fn && (ext = ap_getword(r->pool, &fn, '.'))) {
- extension_info *exinfo;
+
int found;
if (*ext == '\0') /* ignore empty extensions "bad..html" */
----- Original Message -----
From: "Greg Ames" <gr...@remulak.net>
To: <ne...@apache.org>
Sent: Tuesday, August 14, 2001 3:50 PM
Subject: [PATCH] for mod_mime seg fault in 2.0.23 :-)
> Bill Stoddard wrote:
> >
>
> > The problem is that we are creating an extension_info out of a subrequest pool and
> > sticking it into a longer lived hash table (allocated out of the request pool I'm
> > guessing). When we pull the extension_info out of the hash table later and try to use
it,
> > we seg fault because the storage has been reused for something else. No thoughts on a
fix.
>
> This fixes the seg fault on my Linux box:
>
> Index: modules/http/mod_mime.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
> retrieving revision 1.50
> diff -u -d -b -r1.50 mod_mime.c
> --- modules/http/mod_mime.c 2001/08/14 02:35:55 1.50
> +++ modules/http/mod_mime.c 2001/08/14 19:39:45
> @@ -220,13 +220,10 @@
> attrib_info *suffix;
>
> if (base->extension_mappings && add->extension_mappings) {
> - if (base->copy_mappings)
> - new->extension_mappings = base->extension_mappings;
> - else {
> new->extension_mappings = apr_hash_make(p);
> overlay_extension_mappings(p, base->extension_mappings,
> new->extension_mappings);
> - }
> +
> overlay_extension_mappings(p, add->extension_mappings,
> new->extension_mappings);
>
> We can't allow overlay_extension_mappings to store things into the
> extension_mappings hash table which have a shorter lifetime than the
> hash table itself. Always using a new extension_mappings prevents this
> from happening.
>
> I'll commit this in a little while unless someone objects soon.
>
> Greg
> Greg
>
Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by Jeff Trawick <tr...@attglobal.net>.
Greg Ames <gr...@remulak.net> writes:
> This fixes the seg fault on my Linux box:
>
> Index: modules/http/mod_mime.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
> retrieving revision 1.50
> diff -u -d -b -r1.50 mod_mime.c
> --- modules/http/mod_mime.c 2001/08/14 02:35:55 1.50
> +++ modules/http/mod_mime.c 2001/08/14 19:39:45
> @@ -220,13 +220,10 @@
> attrib_info *suffix;
>
> if (base->extension_mappings && add->extension_mappings) {
> - if (base->copy_mappings)
> - new->extension_mappings = base->extension_mappings;
> - else {
> new->extension_mappings = apr_hash_make(p);
> overlay_extension_mappings(p, base->extension_mappings,
> new->extension_mappings);
> - }
> +
> overlay_extension_mappings(p, add->extension_mappings,
> new->extension_mappings);
I suggest committing it. Sure, Bill**2 are suggesting more extensive
changes but this works in the meantime. Even if "in the meantime"
isn't very long, we know that what was in CVS before the
more-extensive changes was working; also, reviewing the more extensive
changes will be a comparison of working with working instead of
working with broken, for what that is worth.
--
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
[PATCH] for mod_mime seg fault in 2.0.23 :-)
Posted by Greg Ames <gr...@remulak.net>.
Bill Stoddard wrote:
>
> The problem is that we are creating an extension_info out of a subrequest pool and
> sticking it into a longer lived hash table (allocated out of the request pool I'm
> guessing). When we pull the extension_info out of the hash table later and try to use it,
> we seg fault because the storage has been reused for something else. No thoughts on a fix.
This fixes the seg fault on my Linux box:
Index: modules/http/mod_mime.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
retrieving revision 1.50
diff -u -d -b -r1.50 mod_mime.c
--- modules/http/mod_mime.c 2001/08/14 02:35:55 1.50
+++ modules/http/mod_mime.c 2001/08/14 19:39:45
@@ -220,13 +220,10 @@
attrib_info *suffix;
if (base->extension_mappings && add->extension_mappings) {
- if (base->copy_mappings)
- new->extension_mappings = base->extension_mappings;
- else {
new->extension_mappings = apr_hash_make(p);
overlay_extension_mappings(p, base->extension_mappings,
new->extension_mappings);
- }
+
overlay_extension_mappings(p, add->extension_mappings,
new->extension_mappings);
We can't allow overlay_extension_mappings to store things into the
extension_mappings hash table which have a shorter lifetime than the
hash table itself. Always using a new extension_mappings prevents this
from happening.
I'll commit this in a little while unless someone objects soon.
Greg
Greg
Re: seg fault in 2.0.23 :-(
Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Mon, Aug 13, 2001 at 08:26:12PM -0400, Bill Stoddard wrote:
> > I was just able to recreate this on my Windows machine. Will post an analysys (and
perhaps
> > a fix) later on.
>
> I have a potential fix but no ability to recreate the fault. ;-)
> Given the butt-ugly nature of the current code, I'll commit what I have and
> you can take it from there.
>
> ....Roy
Still broken after your commit.
The problem is that we are creating an extension_info out of a subrequest pool and
sticking it into a longer lived hash table (allocated out of the request pool I'm
guessing). When we pull the extension_info out of the hash table later and try to use it,
we seg fault because the storage has been reused for something else. No thoughts on a fix.
Bill
Re: seg fault in 2.0.23 :-(
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
On Mon, Aug 13, 2001 at 08:26:12PM -0400, Bill Stoddard wrote:
> I was just able to recreate this on my Windows machine. Will post an analysys (and perhaps
> a fix) later on.
I have a potential fix but no ability to recreate the fault. ;-)
Given the butt-ugly nature of the current code, I'll commit what I have and
you can take it from there.
....Roy
Re: seg fault in 2.0.23 :-(
Posted by Bill Stoddard <bi...@wstoddard.com>.
I was just able to recreate this on my Windows machine. Will post an analysys (and perhaps
a fix) later on.
Bill
> r->content_type is NULL. I don't know why it is NULL at this point.
> All I can say is that the code surrounding that statement SUCKS and
> it has been there for over a year. Someone must have fixed another
> bug that was hiding this one.
>
> ....Roy
>
> On Sat, Aug 11, 2001 at 03:12:29PM -0400, Greg Ames wrote:
> > I put a new 2.0.23 build up very briefly on daedalus, but had to take it
> > down shortly after due to taking a bunch of seg faults. The dump is
> > /usr/local/apache2_0_23/corefiles/httpd.core.1 if anybody wants to have
> > a look. I'm pretty burned out at the moment.
> >
> > find_ct() at line 920 in mod_mime is calling apr_strcat with a bad
> > charset ptr. It's an autoindex request for
> > httpd.apache.org/dist/httpd/old/, running a subrequest on
> > apache_1.3.1.tar.Z to figure out which icon to put on the line, I
> > imagine. find_ct() recently had optimizations done.
> >
> > (gdb) list
> > 915 NULL);
> > 916 }
> > 917 pp = pp->next;
> > 918 }
> > 919 if (charset && !override) {
> > 920 r->content_type = apr_pstrcat(r->pool,
> > r->content_type,
> > 921 "; charset=",
> > charset,
> > 922 NULL);
> > 923 }
> > 924 }
> > (gdb) p charset
> > $4 = 0x2 <Address 0x2 out of bounds>
> > (gdb) bt
> > #0 0x80818a6 in apr_pstrcat (a=0x81ea00c) at apr_strings.c:122
> > #1 0x281b1274 in find_ct (r=0x81ea03c) at mod_mime.c:920
> > #2 0x8070bd4 in ap_run_type_checker (r=0x81ea03c) at request.c:112
> > #3 0x8072054 in ap_sub_req_lookup_dirent (dirent=0xbfbfd7c8,
> > r=0x815003c,
> > next_filter=0x0) at request.c:1458
> > #4 0x281cd1e0 in make_autoindex_entry (dirent=0xbfbfd7c8,
> > autoindex_opts=8196, d=0x81e355c, r=0x815003c, keyid=78 'N',
> > direction=65 'A', pattern=0x0) at mod_autoindex.c:1260
> > #5 0x281cea6b in index_directory (r=0x815003c,
> > autoindex_conf=0x81e355c)
> > at mod_autoindex.c:1951
> > #6 0x281cec22 in handle_autoindex (r=0x815003c) at mod_autoindex.c:2013
> > #7 0x8062520 in ap_run_handler (r=0x815003c) at config.c:185
> > #8 0x806299b in ap_invoke_handler (r=0x815003c) at config.c:344
> > #9 0x806006c in process_request_internal (r=0x815003c) at
> > http_request.c:378#10 0x806014a in ap_process_request (r=0x815003c) at
> > http_request.c:444
> > #11 0x805c31a in ap_process_http_connection (c=0x814a114) at
> > http_core.c:287
> > #12 0x8069504 in ap_run_process_connection (c=0x814a114) at
> > connection.c:82
> > #13 0x8069688 in ap_process_connection (c=0x814a114) at connection.c:219
> > #14 0x8061226 in child_main (child_num_arg=47) at prefork.c:814
> > #15 0x806136e in make_child (s=0x80c2554, slot=47) at prefork.c:901
> > #16 0x80613ce in startup_children (number_to_start=50) at prefork.c:924
> > #17 0x8061727 in ap_mpm_run (_pconf=0x80c200c, plog=0x80f200c,
> > s=0x80c2554)
> > at prefork.c:1139
> >
> > Greg
>
Re: seg fault in 2.0.23 :-(
Posted by Ryan Bloom <rb...@covalent.net>.
I take that back. This is very similar to what the old code does.
I guess just looking at the code isn't going to tell me what is
going on.
Ryan
On Monday 13 August 2001 15:20, Ryan Bloom wrote:
> Wild guess here, but look at line 827. and all the rest of the if
> statements around it. Shouldn't all of those
>
> ((type = exinfo->forced_type)))
>
> be
>
> ((type == exinfo->forced_type)))
>
> I would bet that if they were, we would never hit that apr_pstrcat.
>
> Ryan
>
> On Monday 13 August 2001 15:04, Roy T. Fielding wrote:
> > r->content_type is NULL. I don't know why it is NULL at this point.
> > All I can say is that the code surrounding that statement SUCKS and
> > it has been there for over a year. Someone must have fixed another
> > bug that was hiding this one.
> >
> > ....Roy
> >
> > On Sat, Aug 11, 2001 at 03:12:29PM -0400, Greg Ames wrote:
> > > I put a new 2.0.23 build up very briefly on daedalus, but had to take
> > > it down shortly after due to taking a bunch of seg faults. The dump is
> > > /usr/local/apache2_0_23/corefiles/httpd.core.1 if anybody wants to have
> > > a look. I'm pretty burned out at the moment.
> > >
> > > find_ct() at line 920 in mod_mime is calling apr_strcat with a bad
> > > charset ptr. It's an autoindex request for
> > > httpd.apache.org/dist/httpd/old/, running a subrequest on
> > > apache_1.3.1.tar.Z to figure out which icon to put on the line, I
> > > imagine. find_ct() recently had optimizations done.
> > >
> > > (gdb) list
> > > 915 NULL);
> > > 916 }
> > > 917 pp = pp->next;
> > > 918 }
> > > 919 if (charset && !override) {
> > > 920 r->content_type = apr_pstrcat(r->pool,
> > > r->content_type,
> > > 921 "; charset=",
> > > charset,
> > > 922 NULL);
> > > 923 }
> > > 924 }
> > > (gdb) p charset
> > > $4 = 0x2 <Address 0x2 out of bounds>
> > > (gdb) bt
> > > #0 0x80818a6 in apr_pstrcat (a=0x81ea00c) at apr_strings.c:122
> > > #1 0x281b1274 in find_ct (r=0x81ea03c) at mod_mime.c:920
> > > #2 0x8070bd4 in ap_run_type_checker (r=0x81ea03c) at request.c:112
> > > #3 0x8072054 in ap_sub_req_lookup_dirent (dirent=0xbfbfd7c8,
> > > r=0x815003c,
> > > next_filter=0x0) at request.c:1458
> > > #4 0x281cd1e0 in make_autoindex_entry (dirent=0xbfbfd7c8,
> > > autoindex_opts=8196, d=0x81e355c, r=0x815003c, keyid=78 'N',
> > > direction=65 'A', pattern=0x0) at mod_autoindex.c:1260
> > > #5 0x281cea6b in index_directory (r=0x815003c,
> > > autoindex_conf=0x81e355c)
> > > at mod_autoindex.c:1951
> > > #6 0x281cec22 in handle_autoindex (r=0x815003c) at
> > > mod_autoindex.c:2013 #7 0x8062520 in ap_run_handler (r=0x815003c) at
> > > config.c:185
> > > #8 0x806299b in ap_invoke_handler (r=0x815003c) at config.c:344
> > > #9 0x806006c in process_request_internal (r=0x815003c) at
> > > http_request.c:378#10 0x806014a in ap_process_request (r=0x815003c) at
> > > http_request.c:444
> > > #11 0x805c31a in ap_process_http_connection (c=0x814a114) at
> > > http_core.c:287
> > > #12 0x8069504 in ap_run_process_connection (c=0x814a114) at
> > > connection.c:82
> > > #13 0x8069688 in ap_process_connection (c=0x814a114) at
> > > connection.c:219 #14 0x8061226 in child_main (child_num_arg=47) at
> > > prefork.c:814 #15 0x806136e in make_child (s=0x80c2554, slot=47) at
> > > prefork.c:901 #16 0x80613ce in startup_children (number_to_start=50) at
> > > prefork.c:924 #17 0x8061727 in ap_mpm_run (_pconf=0x80c200c,
> > > plog=0x80f200c, s=0x80c2554)
> > > at prefork.c:1139
> > >
> > > Greg
--
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: seg fault in 2.0.23 :-(
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Roy T. Fielding" <fi...@ebuilt.com>
Sent: Monday, August 13, 2001 5:48 PM
Subject: Re: seg fault in 2.0.23 :-(
> Nope, those look legit, although the coding style is poor. This is going
> to require a complete rewrite.
Just some thoughts on mod_mime...
it groks many different things, but only 'succeeds' on content_type?
It's pretty bogus. I'm thinking we need to ...
1. make the mime phase a run-all. The modules can decide if they have anything
to contribute.
2. pull the default language/content type/etc into a mime_core_default function
that is HOOK_LAST. Therefore ...
mod_mime_fileinfo takes things out of extended file attribs where the filesystem
offers some support for charset/language/content type.
mod_mime works over the filename extensions, further identifying the file.
mod_mime_magic fills in the content type by scanning the file, if we still don't
have a clue.
mod_mime_html works over the html headers for good charset/language meta tags.
core mime_default fills in the final, missing info based on per-dir defaults.
Some of these modules are hypothetical, but it does illustrate that the first might
pick up only a charset, mime might figure out html, but the language wouldn't come
in until mod_mime_html.
As long as one hook phase needs to fill in 5 variables (including the handler!) we
need to be run all.
And as I mentioned to Mr. Stoddard in a sideband headscratching, the table args to the
overlay_extensions_mapping function seem reversed. I'm not touching this for a time,
however, since I have my hands full on some other plates.
But I'm back. New Orleans was nice ('n hot), my brother's wedding was wonderful.
Sorry for my absense :)
Bill
Re: seg fault in 2.0.23 :-(
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
On Mon, Aug 13, 2001 at 03:20:42PM -0700, Ryan Bloom wrote:
>
>
> Wild guess here, but look at line 827. and all the rest of the if statements
> around it. Shouldn't all of those
>
> ((type = exinfo->forced_type)))
>
> be
>
> ((type == exinfo->forced_type)))
>
> I would bet that if they were, we would never hit that apr_pstrcat.
Nope, those look legit, although the coding style is poor. This is going
to require a complete rewrite.
....Roy
Re: seg fault in 2.0.23 :-(
Posted by Brian Pane <bp...@pacbell.net>.
Ryan Bloom wrote:
>
>Wild guess here, but look at line 827. and all the rest of the if statements
>around it. Shouldn't all of those
>
>((type = exinfo->forced_type)))
>
>be
>
>((type == exinfo->forced_type)))
>
>I would bet that if they were, we would never hit that apr_pstrcat
>
Using == here would break the code, as it's uninitialized before the
first of these expressions.
--Brian
Re: seg fault in 2.0.23 :-(
Posted by Ryan Bloom <rb...@covalent.net>.
Wild guess here, but look at line 827. and all the rest of the if statements
around it. Shouldn't all of those
((type = exinfo->forced_type)))
be
((type == exinfo->forced_type)))
I would bet that if they were, we would never hit that apr_pstrcat.
Ryan
On Monday 13 August 2001 15:04, Roy T. Fielding wrote:
> r->content_type is NULL. I don't know why it is NULL at this point.
> All I can say is that the code surrounding that statement SUCKS and
> it has been there for over a year. Someone must have fixed another
> bug that was hiding this one.
>
> ....Roy
>
> On Sat, Aug 11, 2001 at 03:12:29PM -0400, Greg Ames wrote:
> > I put a new 2.0.23 build up very briefly on daedalus, but had to take it
> > down shortly after due to taking a bunch of seg faults. The dump is
> > /usr/local/apache2_0_23/corefiles/httpd.core.1 if anybody wants to have
> > a look. I'm pretty burned out at the moment.
> >
> > find_ct() at line 920 in mod_mime is calling apr_strcat with a bad
> > charset ptr. It's an autoindex request for
> > httpd.apache.org/dist/httpd/old/, running a subrequest on
> > apache_1.3.1.tar.Z to figure out which icon to put on the line, I
> > imagine. find_ct() recently had optimizations done.
> >
> > (gdb) list
> > 915 NULL);
> > 916 }
> > 917 pp = pp->next;
> > 918 }
> > 919 if (charset && !override) {
> > 920 r->content_type = apr_pstrcat(r->pool,
> > r->content_type,
> > 921 "; charset=",
> > charset,
> > 922 NULL);
> > 923 }
> > 924 }
> > (gdb) p charset
> > $4 = 0x2 <Address 0x2 out of bounds>
> > (gdb) bt
> > #0 0x80818a6 in apr_pstrcat (a=0x81ea00c) at apr_strings.c:122
> > #1 0x281b1274 in find_ct (r=0x81ea03c) at mod_mime.c:920
> > #2 0x8070bd4 in ap_run_type_checker (r=0x81ea03c) at request.c:112
> > #3 0x8072054 in ap_sub_req_lookup_dirent (dirent=0xbfbfd7c8,
> > r=0x815003c,
> > next_filter=0x0) at request.c:1458
> > #4 0x281cd1e0 in make_autoindex_entry (dirent=0xbfbfd7c8,
> > autoindex_opts=8196, d=0x81e355c, r=0x815003c, keyid=78 'N',
> > direction=65 'A', pattern=0x0) at mod_autoindex.c:1260
> > #5 0x281cea6b in index_directory (r=0x815003c,
> > autoindex_conf=0x81e355c)
> > at mod_autoindex.c:1951
> > #6 0x281cec22 in handle_autoindex (r=0x815003c) at mod_autoindex.c:2013
> > #7 0x8062520 in ap_run_handler (r=0x815003c) at config.c:185
> > #8 0x806299b in ap_invoke_handler (r=0x815003c) at config.c:344
> > #9 0x806006c in process_request_internal (r=0x815003c) at
> > http_request.c:378#10 0x806014a in ap_process_request (r=0x815003c) at
> > http_request.c:444
> > #11 0x805c31a in ap_process_http_connection (c=0x814a114) at
> > http_core.c:287
> > #12 0x8069504 in ap_run_process_connection (c=0x814a114) at
> > connection.c:82
> > #13 0x8069688 in ap_process_connection (c=0x814a114) at connection.c:219
> > #14 0x8061226 in child_main (child_num_arg=47) at prefork.c:814
> > #15 0x806136e in make_child (s=0x80c2554, slot=47) at prefork.c:901
> > #16 0x80613ce in startup_children (number_to_start=50) at prefork.c:924
> > #17 0x8061727 in ap_mpm_run (_pconf=0x80c200c, plog=0x80f200c,
> > s=0x80c2554)
> > at prefork.c:1139
> >
> > Greg
--
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: seg fault in 2.0.23 :-(
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
r->content_type is NULL. I don't know why it is NULL at this point.
All I can say is that the code surrounding that statement SUCKS and
it has been there for over a year. Someone must have fixed another
bug that was hiding this one.
....Roy
On Sat, Aug 11, 2001 at 03:12:29PM -0400, Greg Ames wrote:
> I put a new 2.0.23 build up very briefly on daedalus, but had to take it
> down shortly after due to taking a bunch of seg faults. The dump is
> /usr/local/apache2_0_23/corefiles/httpd.core.1 if anybody wants to have
> a look. I'm pretty burned out at the moment.
>
> find_ct() at line 920 in mod_mime is calling apr_strcat with a bad
> charset ptr. It's an autoindex request for
> httpd.apache.org/dist/httpd/old/, running a subrequest on
> apache_1.3.1.tar.Z to figure out which icon to put on the line, I
> imagine. find_ct() recently had optimizations done.
>
> (gdb) list
> 915 NULL);
> 916 }
> 917 pp = pp->next;
> 918 }
> 919 if (charset && !override) {
> 920 r->content_type = apr_pstrcat(r->pool,
> r->content_type,
> 921 "; charset=",
> charset,
> 922 NULL);
> 923 }
> 924 }
> (gdb) p charset
> $4 = 0x2 <Address 0x2 out of bounds>
> (gdb) bt
> #0 0x80818a6 in apr_pstrcat (a=0x81ea00c) at apr_strings.c:122
> #1 0x281b1274 in find_ct (r=0x81ea03c) at mod_mime.c:920
> #2 0x8070bd4 in ap_run_type_checker (r=0x81ea03c) at request.c:112
> #3 0x8072054 in ap_sub_req_lookup_dirent (dirent=0xbfbfd7c8,
> r=0x815003c,
> next_filter=0x0) at request.c:1458
> #4 0x281cd1e0 in make_autoindex_entry (dirent=0xbfbfd7c8,
> autoindex_opts=8196, d=0x81e355c, r=0x815003c, keyid=78 'N',
> direction=65 'A', pattern=0x0) at mod_autoindex.c:1260
> #5 0x281cea6b in index_directory (r=0x815003c,
> autoindex_conf=0x81e355c)
> at mod_autoindex.c:1951
> #6 0x281cec22 in handle_autoindex (r=0x815003c) at mod_autoindex.c:2013
> #7 0x8062520 in ap_run_handler (r=0x815003c) at config.c:185
> #8 0x806299b in ap_invoke_handler (r=0x815003c) at config.c:344
> #9 0x806006c in process_request_internal (r=0x815003c) at
> http_request.c:378#10 0x806014a in ap_process_request (r=0x815003c) at
> http_request.c:444
> #11 0x805c31a in ap_process_http_connection (c=0x814a114) at
> http_core.c:287
> #12 0x8069504 in ap_run_process_connection (c=0x814a114) at
> connection.c:82
> #13 0x8069688 in ap_process_connection (c=0x814a114) at connection.c:219
> #14 0x8061226 in child_main (child_num_arg=47) at prefork.c:814
> #15 0x806136e in make_child (s=0x80c2554, slot=47) at prefork.c:901
> #16 0x80613ce in startup_children (number_to_start=50) at prefork.c:924
> #17 0x8061727 in ap_mpm_run (_pconf=0x80c200c, plog=0x80f200c,
> s=0x80c2554)
> at prefork.c:1139
>
> Greg