Fix all the server-side SIGQUIT handlers (grumble ... why so many identical
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 15 May 2009 15:56:39 +0000 (15:56 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 15 May 2009 15:56:39 +0000 (15:56 +0000)
copies?) to ensure they really don't run proc_exit/shmem_exit callbacks,
as was intended.  I broke this behavior recently by installing atexit
callbacks without thinking about the one case where we truly don't want
to run those callback functions.  Noted in an example from Dave Page.

src/backend/access/transam/xlog.c
src/backend/postmaster/autovacuum.c
src/backend/postmaster/bgwriter.c
src/backend/postmaster/walwriter.c
src/backend/storage/ipc/ipc.c
src/backend/tcop/postgres.c

index 134405942a170a5b25fb68c20133d8c548c20adb..93f00c9f6c0e483c0853c232aa4cce07e1ecb828 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.339 2009/05/14 21:28:35 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.340 2009/05/15 15:56:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -7790,14 +7790,22 @@ startupproc_quickdie(SIGNAL_ARGS)
    PG_SETMASK(&BlockSig);
 
    /*
-    * DO NOT proc_exit() -- we're here because shared memory may be
-    * corrupted, so we don't want to try to clean up our transaction. Just
-    * nail the windows shut and get out of town.
-    *
+    * We DO NOT want to run proc_exit() callbacks -- we're here because
+    * shared memory may be corrupted, so we don't want to try to clean up our
+    * transaction.  Just nail the windows shut and get out of town.  Now that
+    * there's an atexit callback to prevent third-party code from breaking
+    * things by calling exit() directly, we have to reset the callbacks
+    * explicitly to make this work as intended.
+    */
+   on_exit_reset();
+
+   /*
     * Note we do exit(2) not exit(0).  This is to force the postmaster into a
     * system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
     * backend.  This is necessary precisely because we don't clean up our
-    * shared memory state.
+    * shared memory state.  (The "dead man switch" mechanism in pmsignal.c
+    * should ensure the postmaster sees this as a crash, too, but no harm
+    * in being doubly sure.)
     */
    exit(2);
 }
index 6b250f2d27e0f2f8adccee48cf338f6e13d1067c..a0a6671afffeb575a78808e2335cfaccd869af3a 100644 (file)
@@ -55,7 +55,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.94 2009/03/31 22:12:48 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.95 2009/05/15 15:56:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1337,14 +1337,22 @@ avl_quickdie(SIGNAL_ARGS)
    PG_SETMASK(&BlockSig);
 
    /*
-    * DO NOT proc_exit() -- we're here because shared memory may be
-    * corrupted, so we don't want to try to clean up our transaction. Just
-    * nail the windows shut and get out of town.
-    *
+    * We DO NOT want to run proc_exit() callbacks -- we're here because
+    * shared memory may be corrupted, so we don't want to try to clean up our
+    * transaction.  Just nail the windows shut and get out of town.  Now that
+    * there's an atexit callback to prevent third-party code from breaking
+    * things by calling exit() directly, we have to reset the callbacks
+    * explicitly to make this work as intended.
+    */
+   on_exit_reset();
+
+   /*
     * Note we do exit(2) not exit(0).  This is to force the postmaster into a
     * system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
     * backend.  This is necessary precisely because we don't clean up our
-    * shared memory state.
+    * shared memory state.  (The "dead man switch" mechanism in pmsignal.c
+    * should ensure the postmaster sees this as a crash, too, but no harm
+    * in being doubly sure.)
     */
    exit(2);
 }
index ebeded0212fa1e6104382582c18d06c66717cbe9..7a23c0130da4c6baf4aaf1ef664ae4633dbfd6d8 100644 (file)
@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.57 2009/03/26 22:26:06 petere Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.58 2009/05/15 15:56:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -798,14 +798,22 @@ bg_quickdie(SIGNAL_ARGS)
    PG_SETMASK(&BlockSig);
 
    /*
-    * DO NOT proc_exit() -- we're here because shared memory may be
-    * corrupted, so we don't want to try to clean up our transaction. Just
-    * nail the windows shut and get out of town.
-    *
+    * We DO NOT want to run proc_exit() callbacks -- we're here because
+    * shared memory may be corrupted, so we don't want to try to clean up our
+    * transaction.  Just nail the windows shut and get out of town.  Now that
+    * there's an atexit callback to prevent third-party code from breaking
+    * things by calling exit() directly, we have to reset the callbacks
+    * explicitly to make this work as intended.
+    */
+   on_exit_reset();
+
+   /*
     * Note we do exit(2) not exit(0).  This is to force the postmaster into a
     * system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
     * backend.  This is necessary precisely because we don't clean up our
-    * shared memory state.
+    * shared memory state.  (The "dead man switch" mechanism in pmsignal.c
+    * should ensure the postmaster sees this as a crash, too, but no harm
+    * in being doubly sure.)
     */
    exit(2);
 }
index f509a865328d510f2b0df0fc03f997b17cb3d0c1..414731ebc5a81ee93cc3320e59f456df580be4c4 100644 (file)
@@ -34,7 +34,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/walwriter.c,v 1.5 2009/01/01 17:23:46 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/walwriter.c,v 1.6 2009/05/15 15:56:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -288,14 +288,22 @@ wal_quickdie(SIGNAL_ARGS)
    PG_SETMASK(&BlockSig);
 
    /*
-    * DO NOT proc_exit() -- we're here because shared memory may be
-    * corrupted, so we don't want to try to clean up our transaction. Just
-    * nail the windows shut and get out of town.
-    *
+    * We DO NOT want to run proc_exit() callbacks -- we're here because
+    * shared memory may be corrupted, so we don't want to try to clean up our
+    * transaction.  Just nail the windows shut and get out of town.  Now that
+    * there's an atexit callback to prevent third-party code from breaking
+    * things by calling exit() directly, we have to reset the callbacks
+    * explicitly to make this work as intended.
+    */
+   on_exit_reset();
+
+   /*
     * Note we do exit(2) not exit(0).  This is to force the postmaster into a
     * system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
     * backend.  This is necessary precisely because we don't clean up our
-    * shared memory state.
+    * shared memory state.  (The "dead man switch" mechanism in pmsignal.c
+    * should ensure the postmaster sees this as a crash, too, but no harm
+    * in being doubly sure.)
     */
    exit(2);
 }
index 5969e6e425b151070d159bec8dcb91f4644d069b..5fa2d5d37cae4d279f4dd1016a1fb3b6e85958f3 100644 (file)
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/ipc/ipc.c,v 1.103 2009/05/05 20:06:07 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/ipc/ipc.c,v 1.104 2009/05/15 15:56:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -166,7 +166,8 @@ proc_exit_prepare(int code)
    /* do our shared memory exits first */
    shmem_exit(code);
 
-   elog(DEBUG3, "proc_exit(%d)", code);
+   elog(DEBUG3, "proc_exit(%d): %d callbacks to make",
+        code, on_proc_exit_index);
 
    /*
     * call all the registered callbacks.
@@ -193,7 +194,8 @@ proc_exit_prepare(int code)
 void
 shmem_exit(int code)
 {
-   elog(DEBUG3, "shmem_exit(%d)", code);
+   elog(DEBUG3, "shmem_exit(%d): %d callbacks to make",
+        code, on_shmem_exit_index);
 
    /*
     * call all the registered callbacks.
index 64295a7bc91dfb67f1f952cd481fd14a5130f1a1..c84fea522a8b01fb5529b90d9c28a74802bb758c 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.565 2009/01/07 19:35:43 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.566 2009/05/15 15:56:39 tgl Exp $
  *
  * NOTES
  *   this is the "main" module of the postgres backend and
@@ -2495,14 +2495,22 @@ quickdie(SIGNAL_ARGS)
                     " database and repeat your command.")));
 
    /*
-    * DO NOT proc_exit() -- we're here because shared memory may be
-    * corrupted, so we don't want to try to clean up our transaction. Just
-    * nail the windows shut and get out of town.
-    *
+    * We DO NOT want to run proc_exit() callbacks -- we're here because
+    * shared memory may be corrupted, so we don't want to try to clean up our
+    * transaction.  Just nail the windows shut and get out of town.  Now that
+    * there's an atexit callback to prevent third-party code from breaking
+    * things by calling exit() directly, we have to reset the callbacks
+    * explicitly to make this work as intended.
+    */
+   on_exit_reset();
+
+   /*
     * Note we do exit(2) not exit(0).  This is to force the postmaster into a
     * system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
     * backend.  This is necessary precisely because we don't clean up our
-    * shared memory state.
+    * shared memory state.  (The "dead man switch" mechanism in pmsignal.c
+    * should ensure the postmaster sees this as a crash, too, but no harm
+    * in being doubly sure.)
     */
    exit(2);
 }
@@ -2554,7 +2562,7 @@ die(SIGNAL_ARGS)
 void
 authdie(SIGNAL_ARGS)
 {
-   exit(1);
+   proc_exit(1);
 }
 
 /*