Fix SPI error cleanup and memory leak

Since the SPI stack has been moved from TopTransactionContext to
TopMemoryContext, setting _SPI_stack to NULL in AtEOXact_SPI() leaks
memory.  In fact, we don't need to do that anymore: We just leave the
allocated stack around for the next SPI use.

Also, refactor the SPI cleanup so that it is run both at transaction end
and when returning to the main loop on an exception.  The latter is
necessary when a procedure calls a COMMIT or ROLLBACK command that
itself causes an error.

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 08f6f67a15c89f822cbfdcb22a85f63735765f70..22dd55c37838bce1371cf254d8a79b02347fddc5 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -260,35 +260,37 @@ SPI_rollback(void)
    _SPI_current->internal_xact = false;
 }
 
+/*
+ * Clean up SPI state.  Called on transaction end (of non-SPI-internal
+ * transactions) and when returning to the main loop on error.
+ */
+void
+SPICleanup(void)
+{
+   _SPI_current = NULL;
+   _SPI_connected = -1;
+   SPI_processed = 0;
+   SPI_lastoid = InvalidOid;
+   SPI_tuptable = NULL;
+}
+
 /*
  * Clean up SPI state at transaction commit or abort.
  */
 void
 AtEOXact_SPI(bool isCommit)
 {
-   /*
-    * Do nothing if the transaction end was initiated by SPI.
-    */
+   /* Do nothing if the transaction end was initiated by SPI. */
    if (_SPI_current && _SPI_current->internal_xact)
        return;
 
-   /*
-    * Note that memory contexts belonging to SPI stack entries will be freed
-    * automatically, so we can ignore them here.  We just need to restore our
-    * static variables to initial state.
-    */
    if (isCommit && _SPI_connected != -1)
        ereport(WARNING,
                (errcode(ERRCODE_WARNING),
                 errmsg("transaction left non-empty SPI stack"),
                 errhint("Check for missing \"SPI_finish\" calls.")));
 
-   _SPI_current = _SPI_stack = NULL;
-   _SPI_stack_depth = 0;
-   _SPI_connected = -1;
-   SPI_processed = 0;
-   SPI_lastoid = InvalidOid;
-   SPI_tuptable = NULL;
+   SPICleanup();
 }
 
 /*

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 3828cae921dc29d8a281a4dd44556936c517635f..f4133953beb0062312b1554a82b53659ac9e58a0 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -42,6 +42,7 @@
 #include "catalog/pg_type.h"
 #include "commands/async.h"
 #include "commands/prepare.h"
+#include "executor/spi.h"
 #include "jit/jit.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
@@ -3941,6 +3942,7 @@ PostgresMain(int argc, char *argv[],
            WalSndErrorCleanup();
 
        PortalErrorCleanup();
+       SPICleanup();
 
        /*
         * We can't release replication slots inside AbortTransaction() as we

diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index e5bdaecc4e3bcca99cabd96a881a93314d12d1d4..143a89a16c45ece119c8a0d8c2b7b2259554e55c 100644 (file)
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -163,6 +163,7 @@ extern void SPI_start_transaction(void);
 extern void SPI_commit(void);
 extern void SPI_rollback(void);
 
+extern void SPICleanup(void);
 extern void AtEOXact_SPI(bool isCommit);
 extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid);