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/05/30 19:41:01 UTC
Reviewing the exit() calls in 2.0
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
-------------------------------------------------------------------------------
RE: Reviewing the exit() calls in 2.0
Posted by "William A. Rowe, Jr." <wr...@lnd.com>.
> From: rbb@covalent.net [mailto:rbb@covalent.net]
> Sent: Tuesday, May 30, 2000 12:41 PM
>
> 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.
Thank you for your *VERY* thorough review of these scenarios. The
Win32 truly cares, since it is hanging on to a whole lot more than
some memory pools that will go away in any case (the winsock interface,
the service control manager, monitor win threads etc...
The group that scared me most was certainly mod_proxy... it sure
doesn't make sense that a module should be able to knock off the
server, but you clarified (most of) the points, and it will give
me a chance to eyeball the worst offenders you've identified.
Don't let that stop anyone else from picking them off the list,
though :-)
> ./src/os/win32/service.c:272: exit(0);
>
> Haven't got a clue what is going on here, somebody else want
> to comment?
Everything in service.c I've recoded, and winnt.c (mpm) I am
reviewing, so I'll be catching anything that is nonsensical
(I hope!)
Kudos! Spared me from updating this issue in STATUS (I'll be
reviewing all the potential lifetime pool allocation issues at
the same time)... as I suggested, there "appeared" to be a
showstopper issue here.
Bill
Re: Reviewing the exit() calls in 2.0
Posted by Greg Stein <gs...@lyra.org>.
On Tue, 30 May 2000 rbb@covalent.net wrote:
> 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.
While many of these exit() calls are "fine", it would be nice to see them
disappear in favor of a structured termination. Rather than just bailing
form the process, it would be nice to see the stack unwind instead.
Unwinding rather than exiting is "better" from a long-term standpoint.
When the code is repurposed, shifted around, or different structures put
into place, there won't be a surprise exit() in there. Also, consider the
case where (say) an MPM allocates some shared memory. If the system then
bails with an exit() because a module could be loaded, then we could end
up leaking shared memory. A structured exit would allow the MPM to dealloc
the shared mem.
Basically: it would be a Good Thing to continue reducing our use of
exit().
Cheers,
-g
--
Greg Stein, http://www.lyra.org/