OSDN Git Service

Clean up some problems in error recovery --- elog() was pretty broken
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 22 Nov 1999 02:06:31 +0000 (02:06 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 22 Nov 1999 02:06:31 +0000 (02:06 +0000)
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
src/backend/utils/error/elog.c
src/include/storage/ipc.h

index eca7490..5c4dc50 100644 (file)
@@ -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 <sys/file.h>
 #include <errno.h>
 
-
 #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 */
 #include <sys/ipc.h>
 #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;
index b8da267..781640f 100644 (file)
@@ -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);
        }
 
index a123448..1293eed 100644 (file)
@@ -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);