From 316c4c57e2b18cb62d73c38f4331f6f7def75385 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
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 eca74905b2c..5c4dc5052c6 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 <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 */
@@ -43,6 +43,13 @@
#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;
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index b8da26779b6..781640fac28 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 a123448e2a5..1293eedde6f 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);
--
GitLab