From b04b32a543763a8eade2fa2aad87ac42059f1543 Mon Sep 17 00:00:00 2001
From: Bruce Momjian <bruce@momjian.us>
Date: Tue, 15 Apr 2008 20:28:47 +0000
Subject: [PATCH] Revert addition of pg_terminate_backend() because of race
 conditions.

---
 doc/TODO                      | 11 ++++++-
 doc/src/FAQ/TODO.html         | 10 +++++-
 doc/src/sgml/func.sgml        | 17 ++--------
 doc/src/sgml/runtime.sgml     |  7 ----
 src/backend/tcop/postgres.c   | 10 +-----
 src/backend/utils/adt/misc.c  | 61 ++++++++---------------------------
 src/include/catalog/pg_proc.h |  2 --
 src/include/storage/proc.h    |  2 --
 src/include/utils/builtins.h  |  1 -
 9 files changed, 36 insertions(+), 85 deletions(-)

diff --git a/doc/TODO b/doc/TODO
index c174260ca8..36f46113e2 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -20,8 +20,17 @@ http://developer.postgresql.org.
 Administration
 ==============
 
-* -Allow administrators to safely terminate individual sessions either
+* Allow administrators to safely terminate individual sessions either
   via an SQL function or SIGTERM
+
+  Lock table corruption following SIGTERM of an individual backend
+  has been reported in 8.0.  A possible cause was fixed in 8.1, but
+  it is unknown whether other problems exist.  This item mostly
+  requires additional testing rather than of writing any new code.
+
+  http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php
+  http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php
+
 * Check for unreferenced table files created by transactions that were
   in-progress when the server terminated abruptly
 
diff --git a/doc/src/FAQ/TODO.html b/doc/src/FAQ/TODO.html
index 59ac9949fe..6872320c74 100644
--- a/doc/src/FAQ/TODO.html
+++ b/doc/src/FAQ/TODO.html
@@ -26,8 +26,16 @@ first.  There is also a developer's wiki at<br/>
 <h1><a name="section_2">Administration</a></h1>
 
 <ul>
-  <li>-<em>Allow administrators to safely terminate individual sessions either</em>
+  <li>Allow administrators to safely terminate individual sessions either
   via an SQL function or SIGTERM
+<p>  Lock table corruption following SIGTERM of an individual backend
+  has been reported in 8.0.  A possible cause was fixed in 8.1, but
+  it is unknown whether other problems exist.  This item mostly
+  requires additional testing rather than of writing any new code.
+</p>
+<p>  <a href="http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php">http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php</a>
+  <a href="http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php">http://archives.postgresql.org/pgsql-hackers/2007-04/msg00218.php</a>
+</p>
   </li><li>Check for unreferenced table files created by transactions that were
   in-progress when the server terminated abruptly
 <p>  <a href="http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php">http://archives.postgresql.org/pgsql-patches/2006-06/msg00096.php</a>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 4f16d90838..b1edec5e36 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -11848,9 +11848,6 @@ SELECT set_config('log_statement_stats', 'off', false);
    <indexterm>
     <primary>pg_cancel_backend</primary>
    </indexterm>
-   <indexterm>
-    <primary>pg_terminate_backend</primary>
-   </indexterm>
    <indexterm>
     <primary>pg_reload_conf</primary>
    </indexterm>
@@ -11886,13 +11883,6 @@ SELECT set_config('log_statement_stats', 'off', false);
        <entry><type>boolean</type></entry>
        <entry>Cancel a backend's current query</entry>
       </row>
-      <row>
-       <entry>
-        <literal><function>pg_terminate_backend</function>(<parameter>pid</parameter> <type>int</>)</literal>
-        </entry>
-       <entry><type>boolean</type></entry>
-       <entry>Terminate a backend</entry>
-      </row>
       <row>
        <entry>
         <literal><function>pg_reload_conf</function>()</literal>
@@ -11917,10 +11907,9 @@ SELECT set_config('log_statement_stats', 'off', false);
    </para>
 
    <para>
-    <function>pg_cancel_backend</> and <function>pg_terminate_backend</>
-    send a query cancel (<systemitem>SIGINT</>) signal to a backend process
-    identified by process ID.  The
-    process ID of an active backend can be found from
+    <function>pg_cancel_backend</> sends a query cancel
+    (<systemitem>SIGINT</>) signal to a backend process identified by
+    process ID.  The process ID of an active backend can be found from
     the <structfield>procpid</structfield> column in the
     <structname>pg_stat_activity</structname> view, or by listing the
     <command>postgres</command> processes on the server with
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 8ead9e6ac7..a64c6f148a 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1372,13 +1372,6 @@ $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput
     well.
    </para>
   </important>
-
-  <para>
-   To terminate a session while allowing other sessions to continue, use
-   <function>pg_terminate_backend()</> (<xref
-   linkend="functions-admin-signal-table">) rather than sending a signal
-   to the child process.
-  </para>
  </sect1>
 
  <sect1 id="preventing-server-spoofing">
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2c7ab98ae6..56efa4cbf7 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2541,8 +2541,7 @@ StatementCancelHandler(SIGNAL_ARGS)
 		 * waiting for input, however.
 		 */
 		if (ImmediateInterruptOK && InterruptHoldoffCount == 0 &&
-			CritSectionCount == 0 &&
-			(!DoingCommandRead || MyProc->terminate))
+			CritSectionCount == 0 && !DoingCommandRead)
 		{
 			/* bump holdoff count to make ProcessInterrupts() a no-op */
 			/* until we are done getting ready for it */
@@ -2622,10 +2621,6 @@ ProcessInterrupts(void)
 			ereport(ERROR,
 					(errcode(ERRCODE_QUERY_CANCELED),
 					 errmsg("canceling autovacuum task")));
-		else if (MyProc->terminate)
-			ereport(ERROR,
-					(errcode(ERRCODE_ADMIN_SHUTDOWN),
-					 errmsg("terminating backend due to administrator command")));
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_QUERY_CANCELED),
@@ -3464,9 +3459,6 @@ PostgresMain(int argc, char *argv[], const char *username)
 		/* We don't have a transaction command open anymore */
 		xact_started = false;
 
-		if (MyProc->terminate)
-			die(SIGINT);
-
 		/* Now we can allow interrupts again */
 		RESUME_INTERRUPTS();
 	}
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index d877977014..0d58053969 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -27,7 +27,6 @@
 #include "postmaster/syslogger.h"
 #include "storage/fd.h"
 #include "storage/pmsignal.h"
-#include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/builtins.h"
 #include "tcop/tcopprot.h"
@@ -90,7 +89,7 @@ current_query(PG_FUNCTION_ARGS)
  * Functions to send signals to other backends.
  */
 static bool
-pg_signal_check(int pid)
+pg_signal_backend(int pid, int sig)
 {
 	if (!superuser())
 		ereport(ERROR,
@@ -107,16 +106,7 @@ pg_signal_check(int pid)
 				(errmsg("PID %d is not a PostgreSQL server process", pid)));
 		return false;
 	}
-	else
-		return true;
-}
 
-/*
- * Functions to send signals to other backends.
- */
-static bool
-pg_signal_backend(int pid, int sig)
-{
 	/* If we have setsid(), signal the backend's whole process group */
 #ifdef HAVE_SETSID
 	if (kill(-pid, sig))
@@ -135,43 +125,7 @@ pg_signal_backend(int pid, int sig)
 Datum
 pg_cancel_backend(PG_FUNCTION_ARGS)
 {
-	int pid = PG_GETARG_INT32(0);
-	
-	if (pg_signal_check(pid))
-		PG_RETURN_BOOL(pg_signal_backend(pid, SIGINT));
-	else
-		PG_RETURN_BOOL(false);
-}
-
-/*
- *	To cleanly terminate a backend, we set PGPROC(pid)->terminate
- *	then send a cancel signal.  We get ProcArrayLock only when
- *	setting PGPROC->terminate so the function might fail in
- *	several places, but that is fine because in those cases the
- *	backend is already gone.
- */
-Datum
-pg_terminate_backend(PG_FUNCTION_ARGS)
-{
-	int pid = PG_GETARG_INT32(0);
-	volatile PGPROC *term_proc;
-
-	/* Is this the super-user, and can we find the PGPROC entry for the pid? */
-	if (pg_signal_check(pid) && (term_proc = BackendPidGetProc(pid)) != NULL)
-	{
-		LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-		/* Recheck now that we have the ProcArray lock. */
-		if (term_proc->pid == pid)
-		{
-			term_proc->terminate = true;
-			LWLockRelease(ProcArrayLock);
-			PG_RETURN_BOOL(pg_signal_backend(pid, SIGINT));
-		}
-		else
-			LWLockRelease(ProcArrayLock);
-	}
-
-	PG_RETURN_BOOL(false);
+	PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
 }
 
 Datum
@@ -215,6 +169,17 @@ pg_rotate_logfile(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(true);
 }
 
+#ifdef NOT_USED
+
+/* Disabled in 8.0 due to reliability concerns; FIXME someday */
+Datum
+pg_terminate_backend(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT32(pg_signal_backend(PG_GETARG_INT32(0), SIGTERM));
+}
+#endif
+
+
 /* Function to find out which databases make use of a tablespace */
 
 typedef struct
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index e035a9b390..b1c2f1c8b4 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3157,8 +3157,6 @@ DESCR("is schema another session's temp schema?");
 
 DATA(insert OID = 2171 ( pg_cancel_backend		PGNSP PGUID 12 1 0 f f t f v 1 16 "23" _null_ _null_ _null_ pg_cancel_backend - _null_ _null_ ));
 DESCR("cancel a server process' current query");
-DATA(insert OID = 2096 ( pg_terminate_backend		PGNSP PGUID 12 1 0 f f t f v 1 16 "23" _null_ _null_ _null_ pg_terminate_backend - _null_ _null_ ));
-DESCR("terminate a server process");
 DATA(insert OID = 2172 ( pg_start_backup		PGNSP PGUID 12 1 0 f f t f v 1 25 "25" _null_ _null_ _null_ pg_start_backup - _null_ _null_ ));
 DESCR("prepare for taking an online backup");
 DATA(insert OID = 2173 ( pg_stop_backup			PGNSP PGUID 12 1 0 f f t f v 0 25 "" _null_ _null_ _null_ pg_stop_backup - _null_ _null_ ));
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 6af7fc1e7c..ae7a2c4cb5 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -91,8 +91,6 @@ struct PGPROC
 
 	bool		inCommit;		/* true if within commit critical section */
 
-	bool		terminate;		/* admin requested termination */
-
 	uint8		vacuumFlags;	/* vacuum-related flags, see above */
 
 	/* Info about LWLock the process is currently waiting for, if any. */
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index e6d478ddde..f9d425f7b9 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -416,7 +416,6 @@ extern Datum nonnullvalue(PG_FUNCTION_ARGS);
 extern Datum current_database(PG_FUNCTION_ARGS);
 extern Datum current_query(PG_FUNCTION_ARGS);
 extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
-extern Datum pg_terminate_backend(PG_FUNCTION_ARGS);
 extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
 extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
 extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);
-- 
2.39.5