From 6708e447efb5046c95bdcf900b6da97f56f97ae8 Mon Sep 17 00:00:00 2001
From: Tom Lane
Date: Thu, 31 Aug 2017 15:10:24 -0400
Subject: Clean up shm_mq cleanup.

The logic around shm_mq_detach was a few bricks shy of a load, because
(contrary to the comments for shm_mq_attach) all it did was update the
shared shm_mq state.  That left us leaking a bit of process-local
memory, but much worse, the on_dsm_detach callback for shm_mq_detach
was still armed.  That means that whenever we ultimately detach from
the DSM segment, we'd run shm_mq_detach again for already-detached,
possibly long-dead queues.  This accidentally fails to fail today,
because we only ever re-use a shm_mq's memory for another shm_mq, and
multiple detach attempts on the last such shm_mq are fairly harmless.
But it's gonna bite us someday, so let's clean it up.

To do that, change shm_mq_detach's API so it takes a shm_mq_handle
not the underlying shm_mq.  This makes the callers simpler in most
cases anyway.  Also fix a few places in parallel.c that were just
pfree'ing the handle structs rather than doing proper cleanup.

Back-patch to v10 because of the risk that the revenant shm_mq_detach
callbacks would cause a live bug sometime.  Since this is an API
change, it's too late to do it in 9.6.  (We could make a variant
patch that preserves API, but I'm not excited enough to do that.)

Discussion: https://postgr.es/m/8670.1504192177@sss.pgh.pa.us
---
 src/backend/libpq/pqmq.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

(limited to 'src/backend/libpq')

diff --git a/src/backend/libpq/pqmq.c b/src/backend/libpq/pqmq.c
index 8fbc03819d9..e1a24b62c8f 100644
--- a/src/backend/libpq/pqmq.c
+++ b/src/backend/libpq/pqmq.c
@@ -21,7 +21,6 @@
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
 
-static shm_mq *pq_mq;
 static shm_mq_handle *pq_mq_handle;
 static bool pq_mq_busy = false;
 static pid_t pq_mq_parallel_master_pid = 0;
@@ -56,7 +55,6 @@ void
 pq_redirect_to_shm_mq(dsm_segment *seg, shm_mq_handle *mqh)
 {
 	PqCommMethods = &PqCommMqMethods;
-	pq_mq = shm_mq_get_queue(mqh);
 	pq_mq_handle = mqh;
 	whereToSendOutput = DestRemote;
 	FrontendProtocol = PG_PROTOCOL_LATEST;
@@ -70,7 +68,6 @@ pq_redirect_to_shm_mq(dsm_segment *seg, shm_mq_handle *mqh)
 static void
 pq_cleanup_redirect_to_shm_mq(dsm_segment *seg, Datum arg)
 {
-	pq_mq = NULL;
 	pq_mq_handle = NULL;
 	whereToSendOutput = DestNone;
 }
@@ -135,9 +132,8 @@ mq_putmessage(char msgtype, const char *s, size_t len)
 	 */
 	if (pq_mq_busy)
 	{
-		if (pq_mq != NULL)
-			shm_mq_detach(pq_mq);
-		pq_mq = NULL;
+		if (pq_mq_handle != NULL)
+			shm_mq_detach(pq_mq_handle);
 		pq_mq_handle = NULL;
 		return EOF;
 	}
@@ -148,7 +144,7 @@ mq_putmessage(char msgtype, const char *s, size_t len)
 	 * be generated late in the shutdown sequence, after all DSMs have already
 	 * been detached.
 	 */
-	if (pq_mq == NULL)
+	if (pq_mq_handle == NULL)
 		return 0;
 
 	pq_mq_busy = true;
-- 
cgit v1.2.3