Fix unsafe coding in ReorderBufferCommit().

"iterstate" must be marked volatile since it's changed inside the PG_TRY
block and then used in the PG_CATCH stanza.  Noted by Mark Wilding of
Salesforce.  (We really need to see if we can't get the C compiler to warn
about this.)

Also, reset iterstate to NULL after the mainline ReorderBufferIterTXNFinish
call, to ensure the PG_CATCH block doesn't try to do that a second time.

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 6e668aeef1e1efe1de7e006f8392a2936fa8ebc3..82f83d1450516e488c2e8bbfabf3bcacc39bd673 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1258,7 +1258,7 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
                                        TimestampTz commit_time)
 {
        ReorderBufferTXN *txn;
-       ReorderBufferIterTXNState *iterstate = NULL;
+       ReorderBufferIterTXNState *volatile iterstate = NULL;
        ReorderBufferChange *change;
 
        volatile CommandId command_id = FirstCommandId;
@@ -1303,7 +1303,6 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
 
        PG_TRY();
        {
-
                /*
                 * Decoding needs access to syscaches et al., which in turn use
                 * heavyweight locks and such. Thus we need to have enough state
@@ -1472,7 +1471,9 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
                        }
                }
 
+               /* clean up the iterator */
                ReorderBufferIterTXNFinish(rb, iterstate);
+               iterstate = NULL;
 
                /* call commit callback */
                rb->commit(rb, txn, commit_lsn);
@@ -1639,7 +1640,7 @@ ReorderBufferForget(ReorderBuffer *rb, TransactionId xid, XLogRecPtr lsn)
         */
        if (txn->base_snapshot != NULL && txn->ninvalidations > 0)
        {
-               bool use_subtxn = IsTransactionOrTransactionBlock();
+               bool            use_subtxn = IsTransactionOrTransactionBlock();
 
                if (use_subtxn)
                        BeginInternalSubTransaction("replay");