httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Reviewing the exit() calls in 2.0
Date Tue, 30 May 2000 17:41:01 GMT

OK, after OtherBill suggested that somebody review where we are just
calling exit in 2.0, I did it.  This is a long message that basically
details every call to exit() in the server and gives my opinion on whether
or not it needs to change to something that cleans up after Apache.  None
of these are gospel, just my opinion.  If somebody is looking for
something to do, this is some low-hanging fruit.

IMHO, Apache 2.0 is for the most part acting pretty good with respect to
just calling exit().  The modules could probably be cleaned up a bit, but
there just isn't too much that is wrong in there.

Ryan

./src/lib/apr/lib/apr_pools.c:258:	    exit(1);
./src/lib/apr/lib/apr_pools.c:322:	    exit(1);

In debug code

./src/lib/apr/lib/apr_pools.c:825:		exit(1);

The owning pool is a FREE_POOL, this is bad, and we need to die, NOW!

./src/lib/apr/lib/apr_pools.c:907:	exit(1);
./src/lib/apr/lib/apr_pools.c:1090:	exit(1);
./src/lib/apr/lib/apr_pools.c:1150:	exit(1);
./src/lib/apr/lib/apr_pools.c:1162:	exit(1);

Is in ALLOC_USE_MALLOC block, which should only be used for debugging

./src/lib/apr/misc/win32/rand.c:67:		     exit(EXIT_FAILURE);*/
./src/lib/apr/misc/win32/rand.c:74:		     exit(EXIT_FAILURE);*/

commented out.  Should probably just be removed

./src/lib/apr/threadproc/unix/proc.c:252:                exit(-1);   /* We have big problems,
the child should exit. */
./src/lib/apr/threadproc/unix/proc.c:284:        exit(-1);  /* if we get here, there is a
problem, so exit with an */ 

Both of these represent big problems with the machine, we are killing the 
child process because there is nothing else to do.  The first is because 
we couldn't chdir to the requested directory, the second is an error in execve.

./src/lib/apr/threadproc/unix/procsup.c:66:        exit(0);
./src/lib/apr/threadproc/unix/procsup.c:70:        exit(1);  /* we can't do anything here,
so just exit. */

These are during the detach phase, where exiting is required.

./src/main/http_main.c:200:    exit(process_exit_value);

This is inside destroy_and_exit_process, a VERY valid place to exit().  :-)

./src/main/http_main.c:217:            exit(1);

Couldn't create the root_pool.  Apache needs to die at this point.

./src/main/util.c:1866:	exit(1);
./src/main/util.c:1884:	exit(1);

Trying to execute the server with an invalid username or groupname 
respectively.  We do this VERY early in the server, IMHO better to die 
immediately than the leave the server running with a very bad configuration.

./src/main/util.c:1927:	exit(1);
./src/main/util.c:1934:	exit(1);
./src/main/util.c:1975:	exit(1);
./src/main/util.c:1984:	exit(1);

These are while we are setting up virtual hosts.  This is still while
we are in the parent process, so exit'ing is okay here, rather than try to
keep going with an invalid config.

./src/main/util_uri.c:273:	exit(1);
./src/main/util_uri.c:292:	exit(1);

Done early in the process to ensure that we have a valid REGEX library.  This
whole function _may_ be able to go away now that we are always using PCRE.

./src/main/http_log.c:217:	                 exit(1);
./src/main/http_log.c:253:            exit(1);

Die because we couldn't fork a child for the error log or we couldn't open the
error log directly.  Might want to convert this to destroy_and_exit_process,
but we should be die'ing here, and we should be early in the parent process, so
just exit'ing should be fine.

./src/main/http_log.c:527:        exit(1);

Couldn't write the pid file, we need to die here, and we are still early in the
parent processing, so just exit'ing is really fine.

./src/main/http_log.c:578:    exit(1);

We are just logging an assertion failure here.  This code should almost never
actually be run.

./src/main/http_config.c:397:	exit(1);

Die when trying to open a module that is for the wrong Apache version.

./src/main/http_config.c:415:	    exit(1);

Die when trying to include too many Dynamic Modules.

./src/main/http_config.c:1272:        exit(1);
./src/main/http_config.c:1322:	exit(1);

Syntax error in -C/-c directive (second is for a syntax error in the config
file itself).  What else to do but die.

./src/main/http_config.c:1307:	exit(1);

Couldn't open config file, gotta die.

./src/modules/mpm/dexter/dexter.c:181:    exit(code);

Clean_child_exit for dexter, gotta use exit here.

./src/modules/mpm/dexter/dexter.c:384:	exit(APEXIT_CHILDFATAL);

A child of ours died due to a fatal error, Apache has always die'd when 
this happens.

./src/modules/mpm/dexter/dexter.c:1071:        exit(1);

Couldn't create the pipe_of_death, which means the parent can't talk to the 
children, should probably change to destroy_process_and_exit().

./src/modules/mpm/dexter/dexter.c:1079:        exit(1);

Couldn't make pipe_of_death non-blocking.  Should probably change to use
APR pipes, and to use destroy_process_and_exit().  [The reason to change to
APR pipes, is that this only attempts one method of making the pipe 
non-blocking, but APR has four different methods.]

./src/modules/mpm/dexter/scoreboard.c:153:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:175:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:229:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:236:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:245:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:289:	    exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:297:	    exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:310:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:321:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:329:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:364:	exit(APEXIT_INIT);
./src/modules/mpm/dexter/scoreboard.c:418:	exit(APEXIT_INIT);

All different errors with regard to shared memory.  Just die'ing is OK IMHO.
BTW, dexter should begin to use APR IMHO.  This code is just duplicating
ALOT of what APR already provides for free.

./src/modules/mpm/mpmt_beos/mpmt_beos.c:168:    exit(code);

clean_child_exit().

./src/modules/mpm/mpmt_beos/mpmt_beos.c:415:	exit(APEXIT_CHILDFATAL);

A child of ours died due to a fatal error, Apache has always die'd when 
this happens.

./src/modules/mpm/mpmt_pthread/mpmt_pthread.c:185:    exit(code);

clean_child_exit()

./src/modules/mpm/mpmt_pthread/mpmt_pthread.c:389:	exit(APEXIT_CHILDFATAL);

A child of ours died due to a fatal error, Apache has always die'd when 
this happens.

./src/modules/mpm/mpmt_pthread/mpmt_pthread.c:1092:        exit(1);
./src/modules/mpm/mpmt_pthread/mpmt_pthread.c:1099:        exit(1);

pipe_of_death problems (same as dexter), again needs to become an APR pipe,
and probably use clean_child_exit().

./src/modules/mpm/mpmt_pthread/scoreboard.c:109:        exit(APEXIT_INIT);
./src/modules/mpm/mpmt_pthread/scoreboard.c:118:        exit(APEXIT_INIT);

Couldn't create shared memory for the scoreboard.  Just exit'ing is probably
OK here.

./src/modules/mpm/prefork/prefork.c:226:    exit(code);

clean_child_exit()

./src/modules/mpm/prefork/prefork.c:255:	exit(-1);
./src/modules/mpm/prefork/prefork.c:260:	exit(-1);
./src/modules/mpm/prefork/prefork.c:264:	exit(-1);
./src/modules/mpm/prefork/prefork.c:268:	exit(-1);
./src/modules/mpm/prefork/prefork.c:273:	exit(-1);
./src/modules/mpm/prefork/prefork.c:346:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:352:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:357:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:362:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:366:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:466:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:471:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:483:	    exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:551:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:634:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:701:	exit(APEXIT_INIT);

Accept_mutex_init (a lot of these could go away with the use of APR).  These
are probably okay, because we haven't even started spawning child processes
yet. 

./src/modules/mpm/prefork/prefork.c:810:	exit(APEXIT_INIT);
./src/modules/mpm/prefork/prefork.c:819:	exit(APEXIT_INIT);

Shared memory problems (same as dexter and mpmt_pthread)

./src/modules/mpm/prefork/prefork.c:1810:	exit(APEXIT_CHILDFATAL);

child died with fatal error... see dexter et al.
  
./src/modules/mpm/spmt_os2/spmt_os2.c:1230:	exit(APEXIT_CHILDFATAL);

child died with fatal error... see dexter et al.

./src/modules/mpm/winnt/winnt.c:441:            exit(1);
./src/modules/mpm/winnt/winnt.c:451:            exit(1);
./src/modules/mpm/winnt/winnt.c:1191:	exit(0);

trouble getting data from parent, exit() is okay here, because we also send
a signal to the parent to die properly.

./src/modules/mpm/winnt/winnt.c:1771:                    exit(1);

Couldn't create Completion port, should be using clean_child_exit (which is of 
course not implemented AFAICT).

./src/modules/mpm/winnt/winnt.c:1786:                exit(1);
./src/modules/mpm/winnt/winnt.c:1798:                exit(1);

Parent can't create shutdown and restart events, these are fine as exit()'s.

General notes about MPMs and exit().  Dexter is by far the worst 
offender of just calling exit().  The OS/2 and BeOS MPMs are without a doubt 
the best MPM in this respect.


./src/modules/proxy/proxy_cache.c:225:		    exit(1);

A fork failed, just exit()'ing is fine.

./src/modules/proxy/proxy_cache.c:235:			exit(1);
./src/modules/proxy/proxy_cache.c:243:			exit(1);
./src/modules/proxy/proxy_cache.c:251:			exit(1);

detaching, so exit() is good here.

./src/modules/proxy/proxy_cache.c:255:		    exit(0);

Looks like real problems if we get here.  :-)

./src/modules/proxy/proxy_cache.c:260:		    exit(0);		    

This is just the right thing to do, because we are detaching.

./src/modules/standard/mod_auth_digest.c:272:	exit(1);

Couldn't generate secret key for digest, should clean up before we die though.

./src/modules/standard/mod_cgid.c:497:    exit(-1);   /* We should NEVER get here */

If we get here, we have bigger problems than calling exit.  This is the line 
after an exec

./src/modules/standard/mod_cgid.c:586:            exit(-1);

Should never get here.

./src/modules/standard/mod_log_config.c:1020:            exit(1);

Would be nice to log and cleanup before we exit.

./src/modules/standard/mod_log_config.c:1030:            exit(1);

At least we log a problem with this one, but we should still cleanup

./src/modules/standard/mod_mime.c:317:        exit(1);

Couldn't open Mime types config file, should probably cleanup before we just
exit.

./src/modules/standard/mod_rewrite.c:971:        exit(1);    /* ugly but I can't log anything
yet. This is what */

May not be able to log, but we could cleanup up after ourselves.

./src/modules/standard/mod_rewrite.c:3177:            exit(1);

Can't open reliable pipe, but we can cleanup.

./src/modules/standard/mod_rewrite.c:3187:            exit(1);

Can't open log, but we can cleanup

./src/modules/standard/mod_rewrite.c:3327:        exit(1);

Parent can't create a lock, but it could cleanup.

./src/modules/standard/mod_rewrite.c:3398:            exit(1);

Couldn't fork a child, we should probably just die.

./src/modules/standard/mod_unique_id.c:206:        exit(1);
./src/modules/standard/mod_unique_id.c:213:        exit(1);

mod_unique_id really needs the hostname of the server, if it can't get it,
we should cleanup and die, not just die.

./src/os/beos/beosd.c:79:	exit(1);

detaching.  this is fine.

./src/os/bs2000/bs2login.c:124:	exit(APEXIT_CHILDFATAL);

We couldn't uname, just die.

./src/os/bs2000/bs2login.c:226:	exit(APEXIT_CHILDFATAL);

Couldn't get switch to the correct user, just die.

./src/os/bs2000/bs2login.c:249:	exit(APEXIT_CHILDFATAL);

Authorization failed, just die.

./src/os/bs2000/bs2login.c:282:	    exit(1);

BS2000 implements it's own fork code, This exit is correct.

./src/os/unix/unixd.c:79:	exit(0);
./src/os/unix/unixd.c:84:	exit(1);
./src/os/unix/unixd.c:93:	exit(1);
./src/os/unix/unixd.c:100:	exit(1);
./src/os/unix/unixd.c:113:	exit(1);

All inside detach, these are correct.

./src/os/unix/unixd.c:212:	    exit(1);

Trying to switch UID, this is correct.

./src/os/win32/main_win32.c:173:    exit(0);

cleanup_and_exit

./src/os/win32/main_win32.c:290:        exit(0); /* nothing to clean up */

Ap_initialize failed, we had better just die, this machine is unstable.

./src/os/win32/service.c:272:        exit(0);

Haven't got a clue what is going on here, somebody else want to comment?


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message