Preserve CurrentMemoryContext across notify and sinval interrupts.

ProcessIncomingNotify is called from the main processing loop that
normally runs in MessageContext.  That outer-loop code assumes that
whatever it allocates will be cleaned up when we're done processing
the current client message --- but if we service a notify interrupt,
then whatever gets allocated before the next switch into
MessageContext will be permanently leaked in TopMemoryContext,
because CommitTransactionCommand sets CurrentMemoryContext to
TopMemoryContext.  There are observable leaks associated with
(at least) encoding conversion of incoming queries and parameters
attached to Bind messages.

sinval catchup interrupts have a similar problem.  There might be
others, but I've not identified any other clear cases.

To fix, take care to save and restore CurrentMemoryContext across
the Start/CommitTransactionCommand calls in these functions.

Per bug #18512 from wizardbrony.  Commit to back branches only;
in HEAD, this was dealt with by the riskier but more thoroughgoing
approach in commit 1afe31f03.

Discussion: https://postgr.es/m/3478884.1718656625@sss.pgh.pa.us

diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 409fece1e06f6345659b402a8e6a2f163ad56eea..fff1aad83ed86d8bf4b29186533fbdf82e42daee 100644 (file)
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -2250,6 +2250,8 @@ asyncQueueAdvanceTail(void)
 static void
 ProcessIncomingNotify(bool flush)
 {
+   MemoryContext oldcontext;
+
    /* We *must* reset the flag */
    notifyInterruptPending = false;
 
@@ -2264,14 +2266,21 @@ ProcessIncomingNotify(bool flush)
 
    /*
     * We must run asyncQueueReadAllNotifications inside a transaction, else
-    * bad things happen if it gets an error.
+    * bad things happen if it gets an error.  However, we need to preserve
+    * the caller's memory context (typically MessageContext).
     */
+   oldcontext = CurrentMemoryContext;
+
    StartTransactionCommand();
 
    asyncQueueReadAllNotifications();
 
    CommitTransactionCommand();
 
+   /* Caller's context had better not have been transaction-local */
+   Assert(MemoryContextIsValid(oldcontext));
+   MemoryContextSwitchTo(oldcontext);
+
    /*
     * If this isn't an end-of-command case, we must flush the notify messages
     * to ensure frontend gets them promptly.

diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c
index f585d63e5cdbc12ca3bd293114c3e04603892053..1654149505c85991f00e9894e0b15da8f9a835be 100644 (file)
--- a/src/backend/storage/ipc/sinval.c
+++ b/src/backend/storage/ipc/sinval.c
@@ -17,6 +17,7 @@
 #include "access/xact.h"
 #include "commands/async.h"
 #include "miscadmin.h"
+#include "nodes/memnodes.h"
 #include "storage/ipc.h"
 #include "storage/proc.h"
 #include "storage/sinvaladt.h"
@@ -184,6 +185,7 @@ ProcessCatchupInterrupt(void)
         * can just call AcceptInvalidationMessages() to do this.  If we
         * aren't, we start and immediately end a transaction; the call to
         * AcceptInvalidationMessages() happens down inside transaction start.
+        * Be sure to preserve caller's memory context when we do that.
         *
         * It is awfully tempting to just call AcceptInvalidationMessages()
         * without the rest of the xact start/stop overhead, and I think that
@@ -197,9 +199,14 @@ ProcessCatchupInterrupt(void)
        }
        else
        {
+           MemoryContext oldcontext = CurrentMemoryContext;
+
            elog(DEBUG4, "ProcessCatchupEvent outside transaction");
            StartTransactionCommand();
            CommitTransactionCommand();
+           /* Caller's context had better not have been transaction-local */
+           Assert(MemoryContextIsValid(oldcontext));
+           MemoryContextSwitchTo(oldcontext);
        }
    }
 }