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 eca74905b2c8fc748dcdae5817cb765bb75334cc..5c4dc5052c6234d627c42734d08ee800b89e271e 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 b8da26779b6cb4f8f89bc03d2d41a9133cb833df..781640fac28e8879c0b12e2b145aad220badf408 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 a123448e2a5ca40d11a2a0f204657802c8346bc1..1293eedde6f3f0e2ca36fae89e981c5eed8d3637 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);