From 316c4c57e2b18cb62d73c38f4331f6f7def75385 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 22 Nov 1999 02:06:31 +0000 Subject: [PATCH] Clean up some problems in error recovery --- elog() was pretty broken for the case of errors in backend startup, and proc_exit's method for coping with errors during proc_exit was *completely* busted. Fixed per discussions on pghackers around 11/6/99. --- src/backend/storage/ipc/ipc.c | 94 ++++++++++++++++-------------------------- src/backend/utils/error/elog.c | 38 ++++++++++++----- src/include/storage/ipc.h | 4 +- 3 files changed, 66 insertions(+), 70 deletions(-) diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c index eca74905b2..5c4dc5052c 100644 --- a/src/backend/storage/ipc/ipc.c +++ b/src/backend/storage/ipc/ipc.c @@ -1,4 +1,4 @@ - /*------------------------------------------------------------------------- +/*------------------------------------------------------------------------- * * ipc.c * POSTGRES inter-process communication definitions. @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/storage/ipc/ipc.c,v 1.42 1999/11/06 19:46:57 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/storage/ipc/ipc.c,v 1.43 1999/11/22 02:06:31 tgl Exp $ * * NOTES * @@ -28,8 +28,8 @@ #include #include - #include "postgres.h" + #include "storage/ipc.h" #include "storage/s_lock.h" /* In Ultrix, sem.h and shm.h must be included AFTER ipc.h */ @@ -43,6 +43,13 @@ #include #endif +/* + * This flag is set during proc_exit() to change elog()'s behavior, + * so that an elog() from an on_proc_exit routine cannot get us out + * of the exit procedure. We do NOT want to go back to the idle loop... + */ +bool proc_exit_inprogress = false; + static int UsePrivateMemory = 0; static void IpcMemoryDetach(int status, char *shmaddr); @@ -70,7 +77,8 @@ typedef struct _PrivateMemStruct char *memptr; } PrivateMem; -PrivateMem IpcPrivateMem[16]; +static PrivateMem IpcPrivateMem[16]; + static int PrivateMemoryCreate(IpcMemoryKey memKey, @@ -105,45 +113,34 @@ PrivateMemoryAttach(IpcMemoryId memid) * -cim 2/6/90 * ---------------------------------------------------------------- */ -static int proc_exit_inprogress = 0; - void proc_exit(int code) { - int i; - - TPRINTF(TRACE_VERBOSE, "proc_exit(%d) [#%d]", code, proc_exit_inprogress); - /* - * If proc_exit is called too many times something bad is happening, so - * exit immediately. This is crafted in two if's for a reason. + * Once we set this flag, we are committed to exit. Any elog() will + * NOT send control back to the main loop, but right back here. */ + proc_exit_inprogress = true; - if (++proc_exit_inprogress == 9) - elog(ERROR, "infinite recursion in proc_exit"); - if (proc_exit_inprogress >= 9) - goto exit; - - /* ---------------- - * if proc_exit_inprocess > 1, then it means that we - * are being invoked from within an on_exit() handler - * and so we return immediately to avoid recursion. - * ---------------- - */ - if (proc_exit_inprogress > 1) - return; + TPRINTF(TRACE_VERBOSE, "proc_exit(%d)", code); /* do our shared memory exits first */ shmem_exit(code); /* ---------------- * call all the callbacks registered before calling exit(). + * + * Note that since we decrement on_proc_exit_index each time, + * if a callback calls elog(ERROR) or elog(FATAL) then it won't + * be invoked again when control comes back here (nor will the + * previously-completed callbacks). So, an infinite loop + * should not be possible. * ---------------- */ - for (i = on_proc_exit_index - 1; i >= 0; --i) - (*on_proc_exit_list[i].function) (code, on_proc_exit_list[i].arg); + while (--on_proc_exit_index >= 0) + (*on_proc_exit_list[on_proc_exit_index].function) (code, + on_proc_exit_list[on_proc_exit_index].arg); -exit: TPRINTF(TRACE_VERBOSE, "exit(%d)", code); exit(code); } @@ -154,44 +151,23 @@ exit: * semaphores after a backend dies horribly * ------------------ */ -static int shmem_exit_inprogress = 0; - void shmem_exit(int code) { - int i; - - TPRINTF(TRACE_VERBOSE, "shmem_exit(%d) [#%d]", - code, shmem_exit_inprogress); - - /* - * If shmem_exit is called too many times something bad is happenig, - * so exit immediately. - */ - if (shmem_exit_inprogress > 9) - { - elog(ERROR, "infinite recursion in shmem_exit"); - exit(-1); - } - - /* ---------------- - * if shmem_exit_inprocess is true, then it means that we - * are being invoked from within an on_exit() handler - * and so we return immediately to avoid recursion. - * ---------------- - */ - if (shmem_exit_inprogress++) - return; + TPRINTF(TRACE_VERBOSE, "shmem_exit(%d)", code); /* ---------------- - * call all the callbacks registered before calling exit(). + * call all the registered callbacks. + * + * As with proc_exit(), we remove each callback from the list + * before calling it, to avoid infinite loop in case of error. * ---------------- */ - for (i = on_shmem_exit_index - 1; i >= 0; --i) - (*on_shmem_exit_list[i].function) (code, on_shmem_exit_list[i].arg); + while (--on_shmem_exit_index >= 0) + (*on_shmem_exit_list[on_shmem_exit_index].function) (code, + on_shmem_exit_list[on_shmem_exit_index].arg); on_shmem_exit_index = 0; - shmem_exit_inprogress = 0; } /* ---------------------------------------------------------------- @@ -202,7 +178,7 @@ shmem_exit(int code) * ---------------------------------------------------------------- */ int - on_proc_exit(void (*function) (), caddr_t arg) +on_proc_exit(void (*function) (), caddr_t arg) { if (on_proc_exit_index >= MAX_ON_EXITS) return -1; @@ -223,7 +199,7 @@ int * ---------------------------------------------------------------- */ int - on_shmem_exit(void (*function) (), caddr_t arg) +on_shmem_exit(void (*function) (), caddr_t arg) { if (on_shmem_exit_index >= MAX_ON_EXITS) return -1; diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index b8da26779b..781640fac2 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/error/elog.c,v 1.51 1999/11/16 06:13:36 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/error/elog.c,v 1.52 1999/11/22 02:06:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -30,6 +30,7 @@ #include "libpq/libpq.h" #include "libpq/pqformat.h" #include "miscadmin.h" +#include "storage/ipc.h" #include "storage/proc.h" #include "tcop/tcopprot.h" #include "utils/trace.h" @@ -371,21 +372,38 @@ elog(int lev, const char *fmt, ...) */ if (lev == ERROR || lev == FATAL) { - if (InError) + /* + * If we have not yet entered the main backend loop (ie, we are in + * the postmaster or in backend startup), then go directly to + * proc_exit. The same is true if anyone tries to report an error + * after proc_exit has begun to run. (It's proc_exit's responsibility + * to see that this doesn't turn into infinite recursion!) But in + * the latter case, we exit with nonzero exit code to indicate that + * something's pretty wrong. + */ + if (proc_exit_inprogress || ! Warn_restart_ready) { - /* error reported during error recovery; don't loop forever */ - elog(REALLYFATAL, "elog: error during error recovery, giving up!"); + fflush(stdout); + fflush(stderr); + ProcReleaseSpins(NULL); /* get rid of spinlocks we hold */ + ProcReleaseLocks(); /* get rid of real locks we hold */ + /* XXX shouldn't proc_exit be doing the above?? */ + proc_exit((int) proc_exit_inprogress); } + /* + * Guard against infinite loop from elog() during error recovery. + */ + if (InError) + elog(REALLYFATAL, "elog: error during error recovery, giving up!"); InError = true; + /* + * Otherwise we can return to the main loop in postgres.c. + * In the FATAL case, postgres.c will call proc_exit, but not + * till after completing a standard transaction-abort sequence. + */ ProcReleaseSpins(NULL); /* get rid of spinlocks we hold */ - if (! Warn_restart_ready) - { - /* error reported before there is a main loop to return to */ - elog(REALLYFATAL, "elog: error in postmaster or backend startup, giving up!"); - } if (lev == FATAL) ExitAfterAbort = true; - /* exit to main loop */ siglongjmp(Warn_restart, 1); } diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h index a123448e2a..1293eedde6 100644 --- a/src/include/storage/ipc.h +++ b/src/include/storage/ipc.h @@ -6,7 +6,7 @@ * * Copyright (c) 1994, Regents of the University of California * - * $Id: ipc.h,v 1.36 1999/10/06 21:58:17 vadim Exp $ + * $Id: ipc.h,v 1.37 1999/11/22 02:06:30 tgl Exp $ * * NOTES * This file is very architecture-specific. This stuff should actually @@ -71,6 +71,8 @@ typedef int IpcMemoryId; /* ipc.c */ +extern bool proc_exit_inprogress; + extern void proc_exit(int code); extern void shmem_exit(int code); extern int on_shmem_exit(void (*function) (), caddr_t arg); -- 2.11.0