Clean up backend-exit-time cleanup behavior. Use on_shmem_exit callbacks

to ensure that we have released buffer refcounts and so forth, rather than
putting ad-hoc operations before (some of the calls to) proc_exit.  Add
commentary to discourage future hackers from repeating that mistake.

2 files changed, 98 insertions, 37 deletions

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index cb56f33c83c..6a9ef98f97a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/error/elog.c,v 1.73 2000/12/06 17:25:46 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/error/elog.c,v 1.74 2000/12/18 00:44:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -159,7 +159,7 @@ elog(int lev, const char *fmt, ...)
 		/* this is probably redundant... */
 		if (IsInitProcessingMode())
 			lev = FATAL;
-		if (StopIfError)
+		if (CritSectionCount > 0)
 			lev = STOP;
 	}
 
@@ -445,21 +445,26 @@ elog(int lev, const char *fmt, ...)
 	{
 
 		/*
+		 * For a FATAL error, we let proc_exit clean up and exit.
+		 *
 		 * If we have not yet entered the main backend loop (ie, we are in
-		 * the postmaster or in backend startup), then go directly to
+		 * the postmaster or in backend startup), we also 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)
+		if (lev == FATAL || !Warn_restart_ready || proc_exit_inprogress)
 		{
+			/*
+			 * fflush here is just to improve the odds that we get to see
+			 * the error message, in case things are so hosed that proc_exit
+			 * crashes.  Any other code you might be tempted to add here
+			 * should probably be in an on_proc_exit callback instead.
+			 */
 			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);
 		}
 
@@ -471,13 +476,8 @@ elog(int lev, const char *fmt, ...)
 		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.
+		 * Otherwise we can return to the main loop in postgres.c.
 		 */
-		ProcReleaseSpins(NULL); /* get rid of spinlocks we hold */
-		if (lev == FATAL)
-			ExitAfterAbort = true;
 		siglongjmp(Warn_restart, 1);
 	}
 
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index f213e561c1b..c8bd8a0d266 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.75 2000/12/14 23:51:35 wieck Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/init/postinit.c,v 1.76 2000/12/18 00:44:48 tgl Exp $
  *
  *
  *-------------------------------------------------------------------------
@@ -28,6 +28,7 @@
 #include "access/heapam.h"
 #include "catalog/catname.h"
 #include "catalog/pg_database.h"
+#include "commands/trigger.h"
 #include "miscadmin.h"
 #include "storage/backendid.h"
 #include "storage/proc.h"
@@ -37,6 +38,7 @@
 #include "utils/portal.h"
 #include "utils/relcache.h"
 #include "utils/syscache.h"
+#include "utils/temprel.h"
 
 #ifdef MULTIBYTE
 #include "mb/pg_wchar.h"
@@ -44,6 +46,9 @@
 
 static void ReverifyMyDatabase(const char *name);
 static void InitCommunication(void);
+static void ShutdownPostgres(void);
+
+int			lockingOff = 0;		/* backend -L switch */
 
 
 /*** InitPostgres support ***/
@@ -115,12 +120,8 @@ ReverifyMyDatabase(const char *name)
 	 */
 	dbform = (Form_pg_database) GETSTRUCT(tup);
 	if (! dbform->datallowconn)
-	{
-		heap_endscan(pgdbscan);
-		heap_close(pgdbrel, AccessShareLock);
 		elog(FATAL, "Database \"%s\" is not currently accepting connections",
 			 name);
-	}
 
 	/*
 	 * OK, we're golden.  Only other to-do item is to save the MULTIBYTE
@@ -163,6 +164,28 @@ InitCommunication(void)
 }
 
 
+/*
+ * Early initialization of a backend (either standalone or under postmaster).
+ * This happens even before InitPostgres.
+ */
+void
+BaseInit(void)
+{
+	/*
+	 * Attach to shared memory and semaphores, and initialize our
+	 * input/output/debugging file descriptors.
+	 */
+	InitCommunication();
+	DebugFileOpen();
+
+	/* Do local initialization of storage and buffer managers */
+	smgrinit();
+	InitBufferPoolAccess();
+	InitLocalBuffer();
+
+	EnablePortalManager();		/* memory for portal/transaction stuff */
+}
+
 
 /* --------------------------------
  * InitPostgres
@@ -172,16 +195,13 @@ InitCommunication(void)
  *		Be very careful with the order of calls in the InitPostgres function.
  * --------------------------------
  */
-int			lockingOff = 0;		/* backend -L switch */
-
-/*
- */
 void
 InitPostgres(const char *dbname, const char *username)
 {
 	bool		bootstrap = IsBootstrapProcessingMode();
 
 	SetDatabaseName(dbname);
+
 	/* ----------------
 	 *	initialize the database id used for system caches and lock tables
 	 * ----------------
@@ -299,6 +319,12 @@ InitPostgres(const char *dbname, const char *username)
 	 */
 	InitCatalogCache();
 
+	/*
+	 * Initialize the deferred trigger manager --- must happen before
+	 * first transaction start.
+	 */
+	DeferredTriggerInit();
+
 	/* start a new transaction here before access to db */
 	if (!bootstrap)
 		StartTransactionCommand();
@@ -322,27 +348,62 @@ InitPostgres(const char *dbname, const char *username)
 
 	/*
 	 * Unless we are bootstrapping, double-check that InitMyDatabaseInfo()
-	 * got a correct result.  We can't do this until essentially all the
-	 * infrastructure is up, so just do it at the end.
+	 * got a correct result.  We can't do this until all the database-access
+	 * infrastructure is up.
 	 */
 	if (!bootstrap)
 		ReverifyMyDatabase(dbname);
-}
 
-void
-BaseInit(void)
-{
+#ifdef MULTIBYTE
+	/* set default client encoding --- uses info from ReverifyMyDatabase */
+	set_default_client_encoding();
+#endif
+
 	/*
-	 * Attach to shared memory and semaphores, and initialize our
-	 * input/output/debugging file descriptors.
+	 * Set up process-exit callbacks to remove temp relations and then
+	 * do pre-shutdown cleanup.  This should be last because we want
+	 * shmem_exit to call these routines before the exit callbacks that
+	 * are registered by buffer manager, lock manager, etc.  We need
+	 * to run this code before we close down database access!
 	 */
-	InitCommunication();
-	DebugFileOpen();
-
-	smgrinit();
+	on_shmem_exit(ShutdownPostgres, 0);
+	/* because callbacks are called in reverse order, this gets done first: */
+	on_shmem_exit(remove_all_temp_relations, 0);
 
-	EnablePortalManager();		/* memory for portal/transaction stuff */
+	/* close the transaction we started above */
+	if (!bootstrap)
+		CommitTransactionCommand();
+}
 
-	/* initialize the local buffer manager */
-	InitLocalBuffer();
+/*
+ * Backend-shutdown callback.  Do cleanup that we want to be sure happens
+ * before all the supporting modules begin to nail their doors shut via
+ * their own callbacks.  Note that because this has to be registered very
+ * late in startup, it will not get called if we suffer a failure *during*
+ * startup.
+ *
+ * User-level cleanup, such as temp-relation removal and UNLISTEN, happens
+ * via separate callbacks that execute before this one.  We don't combine the
+ * callbacks because we still want this one to happen if the user-level
+ * cleanup fails.
+ */
+static void
+ShutdownPostgres(void)
+{
+	/*
+	 * These operations are really just a minimal subset of AbortTransaction().
+	 * We don't want to do any inessential cleanup, since that just raises
+	 * the odds of failure --- but there's some stuff we need to do.
+	 *
+	 * Release any spinlocks that we may hold.  This is a kluge to improve
+	 * the odds that we won't get into a self-made stuck spinlock scenario
+	 * while trying to shut down.
+	 */
+	ProcReleaseSpins(NULL);
+	/*
+	 * In case a transaction is open, delete any files it created.  This
+	 * has to happen before bufmgr shutdown, so having smgr register a
+	 * callback for it wouldn't work.
+	 */
+	smgrDoPendingDeletes(false); /* delete as though aborting xact */
 }