You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@bellsouth.net> on 2000/03/22 11:18:24 UTC
[PATCH] one last try to avoid SIGSEGV on startup
The patch below (ignore the uninteresting but valid MULTITHREAD
change) avoids a SIGSEGV if ap_create_context() fails. The
particular condition used to test this code was an inability to
initialize a mutex in ap_init_alloc().
This is basically the same patch that has been tossed around a
few times. Ryan has already applied and/or reimplemented the more
interesting patches from the original set.
Should I:
a) commit (thanks, Dirk!)
b) toss the change and get on with life
c) rework it according to the instructions of someone who has
actually looked at this code
I don't like the idea of bailing out right there myself, but
main() and some of the functions it calls are full of unhandled
error paths, and it doesn't seem important right now to implement
a "proper" error recovery philosophy for paths that may not ever
fail; this is different than most of those other paths because
i) there is no context to clean up
ii) various folks have actually hit this one :)
I'm not claiming that this solves whatever kept ap_create_context()
from working, but I have a strong aversion to SIGSEGV for any
reason and at least we write a log message that is of some small
benefit. (One has to know the way Apache/APR was configured to start
guessing at what might have caused ap_create_context() to fail.)
Thanks for any comments!
===================================================================
RCS file: /cvs/apache/apache-2.0/src/main/http_main.c,v
retrieving revision 1.34
diff -u -r1.34 http_main.c
--- http_main.c 2000/03/15 23:18:31 1.34
+++ http_main.c 2000/03/22 10:01:04
@@ -147,9 +147,6 @@
printf(" -D PIPE_BUF=%ld\n",(long)PIPE_BUF);
#endif
#endif
-#ifdef MULTITHREAD
- printf(" -D MULTITHREAD\n");
-#endif
#ifdef CHARSET_EBCDIC
printf(" -D CHARSET_EBCDIC\n");
#endif
@@ -205,8 +202,16 @@
{
ap_context_t *cntx;
+ ap_status_t stat;
+
+ stat = ap_create_context(&cntx, NULL);
+ if (stat != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, NULL,
+ "ap_create_context() failed to create "
+ "initial context");
+ exit(1);
+ }
- ap_create_context(&cntx, NULL);
process = ap_palloc(cntx, sizeof(process_rec));
process->pool = cntx;
}
--
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] one last try to avoid SIGSEGV on startup
Posted by Chuck Murcko <ch...@topsail.org>.
Jeff Trawick wrote:
>
> The patch below (ignore the uninteresting but valid MULTITHREAD
> change) avoids a SIGSEGV if ap_create_context() fails. The
> particular condition used to test this code was an inability to
> initialize a mutex in ap_init_alloc().
>
+1. Error checking is Good and Righteous.
--
Chuck
Chuck Murcko
Topsail Group
chuck@topsail.org
Re: [PATCH] one last try to avoid SIGSEGV on startup
Posted by rb...@apache.org.
I didn't pay enough attention to the patch. I guess you were removing the
MULTITHREADED, not adding it.
Ryan
On Wed, 22 Mar 2000 rbb@apache.org wrote:
>
> Commit the error condition patch, but leave the Multithreaded stuff alone.
> MUTLITHREADED isn't used in 2.0, and should be removed completely. Sorry
> I missed this portion of the patch BTW.
>
> Ryan
>
>
> > > ===================================================================
> > > RCS file: /cvs/apache/apache-2.0/src/main/http_main.c,v
> > > retrieving revision 1.34
> > > diff -u -r1.34 http_main.c
> > > --- http_main.c 2000/03/15 23:18:31 1.34
> > > +++ http_main.c 2000/03/22 10:01:04
> > > @@ -147,9 +147,6 @@
> > > printf(" -D PIPE_BUF=%ld\n",(long)PIPE_BUF);
> > > #endif
> > > #endif
> > > -#ifdef MULTITHREAD
> > > - printf(" -D MULTITHREAD\n");
> > > -#endif
> > > #ifdef CHARSET_EBCDIC
> > > printf(" -D CHARSET_EBCDIC\n");
> > > #endif
> > > @@ -205,8 +202,16 @@
> > >
> > > {
> > > ap_context_t *cntx;
> > > + ap_status_t stat;
> > > +
> > > + stat = ap_create_context(&cntx, NULL);
> > > + if (stat != APR_SUCCESS) {
> > > + ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, NULL,
> > > + "ap_create_context() failed to create "
> > > + "initial context");
> > > + exit(1);
> > > + }
> > >
> > > - ap_create_context(&cntx, NULL);
> > > process = ap_palloc(cntx, sizeof(process_rec));
> > > process->pool = cntx;
> > > }
> > > --
> > > Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> > > http://www.geocities.com/SiliconValley/Park/9289/
> > > Born in Roswell... married an alien...
> > >
> >
> > --
> > Greg Stein, http://www.lyra.org/
> >
> >
>
>
> _______________________________________________________________________________
> 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] one last try to avoid SIGSEGV on startup
Posted by rb...@apache.org.
Commit the error condition patch, but leave the Multithreaded stuff alone.
MUTLITHREADED isn't used in 2.0, and should be removed completely. Sorry
I missed this portion of the patch BTW.
Ryan
> > ===================================================================
> > RCS file: /cvs/apache/apache-2.0/src/main/http_main.c,v
> > retrieving revision 1.34
> > diff -u -r1.34 http_main.c
> > --- http_main.c 2000/03/15 23:18:31 1.34
> > +++ http_main.c 2000/03/22 10:01:04
> > @@ -147,9 +147,6 @@
> > printf(" -D PIPE_BUF=%ld\n",(long)PIPE_BUF);
> > #endif
> > #endif
> > -#ifdef MULTITHREAD
> > - printf(" -D MULTITHREAD\n");
> > -#endif
> > #ifdef CHARSET_EBCDIC
> > printf(" -D CHARSET_EBCDIC\n");
> > #endif
> > @@ -205,8 +202,16 @@
> >
> > {
> > ap_context_t *cntx;
> > + ap_status_t stat;
> > +
> > + stat = ap_create_context(&cntx, NULL);
> > + if (stat != APR_SUCCESS) {
> > + ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, NULL,
> > + "ap_create_context() failed to create "
> > + "initial context");
> > + exit(1);
> > + }
> >
> > - ap_create_context(&cntx, NULL);
> > process = ap_palloc(cntx, sizeof(process_rec));
> > process->pool = cntx;
> > }
> > --
> > Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> > http://www.geocities.com/SiliconValley/Park/9289/
> > Born in Roswell... married an alien...
> >
>
> --
> Greg Stein, http://www.lyra.org/
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] one last try to avoid SIGSEGV on startup
Posted by Greg Stein <gs...@lyra.org>.
Commit it... +1
(you can't make the behavior worse than its current behavior :-)
Cheers,
-g
On Wed, 22 Mar 2000, Jeff Trawick wrote:
> The patch below (ignore the uninteresting but valid MULTITHREAD
> change) avoids a SIGSEGV if ap_create_context() fails. The
> particular condition used to test this code was an inability to
> initialize a mutex in ap_init_alloc().
>
> This is basically the same patch that has been tossed around a
> few times. Ryan has already applied and/or reimplemented the more
> interesting patches from the original set.
>
> Should I:
>
> a) commit (thanks, Dirk!)
> b) toss the change and get on with life
> c) rework it according to the instructions of someone who has
> actually looked at this code
>
> I don't like the idea of bailing out right there myself, but
> main() and some of the functions it calls are full of unhandled
> error paths, and it doesn't seem important right now to implement
> a "proper" error recovery philosophy for paths that may not ever
> fail; this is different than most of those other paths because
>
> i) there is no context to clean up
> ii) various folks have actually hit this one :)
>
> I'm not claiming that this solves whatever kept ap_create_context()
> from working, but I have a strong aversion to SIGSEGV for any
> reason and at least we write a log message that is of some small
> benefit. (One has to know the way Apache/APR was configured to start
> guessing at what might have caused ap_create_context() to fail.)
>
> Thanks for any comments!
>
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/main/http_main.c,v
> retrieving revision 1.34
> diff -u -r1.34 http_main.c
> --- http_main.c 2000/03/15 23:18:31 1.34
> +++ http_main.c 2000/03/22 10:01:04
> @@ -147,9 +147,6 @@
> printf(" -D PIPE_BUF=%ld\n",(long)PIPE_BUF);
> #endif
> #endif
> -#ifdef MULTITHREAD
> - printf(" -D MULTITHREAD\n");
> -#endif
> #ifdef CHARSET_EBCDIC
> printf(" -D CHARSET_EBCDIC\n");
> #endif
> @@ -205,8 +202,16 @@
>
> {
> ap_context_t *cntx;
> + ap_status_t stat;
> +
> + stat = ap_create_context(&cntx, NULL);
> + if (stat != APR_SUCCESS) {
> + ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, NULL,
> + "ap_create_context() failed to create "
> + "initial context");
> + exit(1);
> + }
>
> - ap_create_context(&cntx, NULL);
> process = ap_palloc(cntx, sizeof(process_rec));
> process->pool = cntx;
> }
> --
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
>
--
Greg Stein, http://www.lyra.org/