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 58b647af5223efe0fbdd6ae795d63638028da6c6..1b575e242a508c80416da974ac0cf129d9bc38f3 100644 (file)
@@ -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 20a5d707486af4085c927ac164965b2e764f30c5..ad7228144f0222b0a3c5043e803049fb6467dcfb 100644 (file)
@@ -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 9d5dd8aa99371dfc1758e007ca9de5ba63f2c0e7..11891778007cd9bedf2e580b6b2c7502390822be 100644 (file)
@@ -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 1760f49bc49c8e379888a5d90a01683c5418ad63..8d78e6dcd85f29825a18544c1fff9520077aa113 100644 (file)
@@ -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 0242f4f916eee93320c88e51c713320a50dddff5..459b4c686dfd322e0dfcc32edbf1d2fbc2bfa918 100644 (file)
@@ -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 3781b55be899c6d65c786ec0e727b14417a1ad4a..c880502f07af778d7df29a7e52726d46e5a05c93 100644 (file)
@@ -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);
 }
 
 /*