diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-08-31 15:10:24 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-08-31 15:10:24 -0400 |
commit | 6708e447efb5046c95bdcf900b6da97f56f97ae8 (patch) | |
tree | e3d331eec9f26181d2a0983f303bd8eb49e9fea8 /src/backend/libpq/pqmq.c | |
parent | 4b1dd62a257a469f92fef4f4cce37beab6c0b98b (diff) | |
download | postgresql-6708e447efb5046c95bdcf900b6da97f56f97ae8.tar.gz postgresql-6708e447efb5046c95bdcf900b6da97f56f97ae8.zip |
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
Diffstat (limited to 'src/backend/libpq/pqmq.c')
-rw-r--r-- | src/backend/libpq/pqmq.c | 10 |
1 files changed, 3 insertions, 7 deletions
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; |