You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2006/05/10 14:04:03 UTC
[PATCH] aborting on OOM
There a few choices for what to do in the oom handler: 1.3 fprintf's to
stderr, then does exit(1), which doesn't seem particularly wise since
fprintf can itself malloc; could do similarly, could just exit(1) or
even just exit(APEXIT_CHILDSICK); but then nothing gets logged. With
abort() at least something is logged, and you can get core dumps with a
suitably configured environment/server, for further diagnosis. Any
opinions?
Index: server/main.c
===================================================================
--- server/main.c (revision 405449)
+++ server/main.c (working copy)
@@ -261,6 +261,13 @@
exit(process_exit_value);
}
+/* APR callback invoked if allocation fails. */
+static int abort_on_oom(int retcode)
+{
+ abort();
+ return retcode; /* unreachable, hopefully. */
+}
+
static process_rec *create_process(int argc, const char * const *argv)
{
process_rec *process;
@@ -279,6 +286,7 @@
exit(1);
}
+ apr_pool_abort_set(abort_on_oom, cntx);
apr_pool_tag(cntx, "process");
ap_open_stderr_log(cntx);
@@ -449,6 +457,10 @@
pconf = process->pconf;
ap_server_argv0 = process->short_name;
+ /* Set up the OOM callback in the global pool, so all pools should
+ * by default inherit it. */
+ apr_pool_abort_set(abort_on_oom, apr_pool_parent_get(process->pool));
+
#if APR_CHARSET_EBCDIC
if (ap_init_ebcdic(pglobal) != APR_SUCCESS) {
destroy_and_exit_process(process, 1);
Re: [PATCH] aborting on OOM
Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Thu, May 11, 2006 at 04:01:50PM +0100, Joe Orton wrote:
> +#if APR_HAVE_UNISTD_H
> +#include <unistd.h>
> +#endif
> +
We might need io.h on win32, but we can easily figure that out :)
> +#define OOM_MESSAGE "[crit] Memory allocation failed, aborting process.\n"
APR_EOL_STR instead of \n, but apart from that, +1
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
Re: [PATCH] aborting on OOM
Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 10, 2006 at 07:11:42PM +0100, Colm MacCarthaigh wrote:
> On Wed, May 10, 2006 at 10:53:50AM -0700, Garrett Rooney wrote:
> > I would personally prefer abort to exit...
>
> is write()'ing a static error message an option too?
I don't know if more effort than this is required to make write()
available on non-Unix platforms, if this is enough I'll commit it:
Index: server/main.c
===================================================================
--- server/main.c (revision 405449)
+++ server/main.c (working copy)
@@ -40,6 +40,10 @@
#include "ap_mpm.h"
#include "mpm_common.h"
+#if APR_HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
/* WARNING: Win32 binds http_main.c dynamically to the server. Please place
* extern functions and global data in another appropriate module.
*
@@ -261,6 +265,16 @@
exit(process_exit_value);
}
+#define OOM_MESSAGE "[crit] Memory allocation failed, aborting process.\n"
+
+/* APR callback invoked if allocation fails. */
+static int abort_on_oom(int retcode)
+{
+ write(STDERR_FILENO, OOM_MESSAGE, strlen(OOM_MESSAGE));
+ abort();
+ return retcode; /* unreachable, hopefully. */
+}
+
static process_rec *create_process(int argc, const char * const *argv)
{
process_rec *process;
@@ -279,6 +293,7 @@
exit(1);
}
+ apr_pool_abort_set(abort_on_oom, cntx);
apr_pool_tag(cntx, "process");
ap_open_stderr_log(cntx);
@@ -449,6 +464,10 @@
pconf = process->pconf;
ap_server_argv0 = process->short_name;
+ /* Set up the OOM callback in the global pool, so all pools should
+ * by default inherit it. */
+ apr_pool_abort_set(abort_on_oom, apr_pool_parent_get(process->pool));
+
#if APR_CHARSET_EBCDIC
if (ap_init_ebcdic(pglobal) != APR_SUCCESS) {
destroy_and_exit_process(process, 1);
Re: [PATCH] aborting on OOM
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Colm MacCarthaigh wrote:
> On Wed, May 10, 2006 at 11:11:27AM -0700, Garrett Rooney wrote:
>
>>On 5/10/06, Colm MacCarthaigh <co...@stdlib.net> wrote:
>>
>>>On Wed, May 10, 2006 at 10:53:50AM -0700, Garrett Rooney wrote:
>>>
>>>>I would personally prefer abort to exit...
>>>
>>>is write()'ing a static error message an option too?
>>
>>Perhaps, but where would you write() to?
>
>
> STDERR_FILENO :)
... is likely to be dead. We shot that horse on startup in most daemons, and
the process may be too unstable to successfully invoke this write :) That said,
it may not hurt to try.
We must continue to segfault, but in a safe and predictable way, to ensure
that the crash dump gives us enough to chew on.
Bill
Re: [PATCH] aborting on OOM
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 5/10/06, Colm MacCarthaigh <co...@stdlib.net> wrote:
> On Wed, May 10, 2006 at 11:22:25AM -0700, Garrett Rooney wrote:
> > Which is likely to be redirected to /dev/null in most cases...
>
> We redirect standard error to the main error log :) See ap_open_logs in
> server/log.c :-) httpd -E also causes stderr redirection for the
> start-up phase, /dev/null is the usually the exception :)
Ahh, I stand corrected. Sounds good to me.
-garrett
Re: [PATCH] aborting on OOM
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Colm MacCarthaigh wrote:
> On Wed, May 10, 2006 at 11:22:25AM -0700, Garrett Rooney wrote:
>
>>Which is likely to be redirected to /dev/null in most cases...
>
> We redirect standard error to the main error log :) See ap_open_logs in
> server/log.c :-) httpd -E also causes stderr redirection for the
> start-up phase, /dev/null is the usually the exception :)
Actually, httpd -E /somewhere/somefile should be used in cases where the child
invoked doesn't have stdout, or it isn't going to the place we want it to.
E.g. on a win32 service, there's no stderr handle created. -E gives us a chance
to inject one :)
Bill
Re: [PATCH] aborting on OOM
Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Wed, May 10, 2006 at 11:22:25AM -0700, Garrett Rooney wrote:
> Which is likely to be redirected to /dev/null in most cases...
We redirect standard error to the main error log :) See ap_open_logs in
server/log.c :-) httpd -E also causes stderr redirection for the
start-up phase, /dev/null is the usually the exception :)
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
Re: [PATCH] aborting on OOM
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 5/10/06, Colm MacCarthaigh <co...@stdlib.net> wrote:
> On Wed, May 10, 2006 at 11:11:27AM -0700, Garrett Rooney wrote:
> > On 5/10/06, Colm MacCarthaigh <co...@stdlib.net> wrote:
> > >On Wed, May 10, 2006 at 10:53:50AM -0700, Garrett Rooney wrote:
> > >> I would personally prefer abort to exit...
> > >
> > >is write()'ing a static error message an option too?
> >
> > Perhaps, but where would you write() to?
>
> STDERR_FILENO :)
Which is likely to be redirected to /dev/null in most cases...
-garrett
Re: [PATCH] aborting on OOM
Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Wed, May 10, 2006 at 11:11:27AM -0700, Garrett Rooney wrote:
> On 5/10/06, Colm MacCarthaigh <co...@stdlib.net> wrote:
> >On Wed, May 10, 2006 at 10:53:50AM -0700, Garrett Rooney wrote:
> >> I would personally prefer abort to exit...
> >
> >is write()'ing a static error message an option too?
>
> Perhaps, but where would you write() to?
STDERR_FILENO :)
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
Re: [PATCH] aborting on OOM
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 5/10/06, Colm MacCarthaigh <co...@stdlib.net> wrote:
> On Wed, May 10, 2006 at 10:53:50AM -0700, Garrett Rooney wrote:
> > I would personally prefer abort to exit...
>
> is write()'ing a static error message an option too?
Perhaps, but where would you write() to?
-garrett
Re: [PATCH] aborting on OOM
Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Wed, May 10, 2006 at 10:53:50AM -0700, Garrett Rooney wrote:
> I would personally prefer abort to exit...
is write()'ing a static error message an option too?
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
Re: [PATCH] aborting on OOM
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 5/10/06, Joe Orton <jo...@redhat.com> wrote:
> There a few choices for what to do in the oom handler: 1.3 fprintf's to
> stderr, then does exit(1), which doesn't seem particularly wise since
> fprintf can itself malloc; could do similarly, could just exit(1) or
> even just exit(APEXIT_CHILDSICK); but then nothing gets logged. With
> abort() at least something is logged, and you can get core dumps with a
> suitably configured environment/server, for further diagnosis. Any
> opinions?
I would personally prefer abort to exit...
-garrett