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 a3bfa637138f91559db9ad85e46ab27cae659191..c7f7fac6c8b1c9f34a68a9f539f8a39f9a2f81d4 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1259,7 +1259,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;
@@ -1304,7 +1304,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
@@ -1473,7 +1472,9 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
            }
        }
 
+       /* clean up the iterator */
        ReorderBufferIterTXNFinish(rb, iterstate);
+       iterstate = NULL;
 
        /* call commit callback */
        rb->commit(rb, txn, commit_lsn);
@@ -1640,7 +1641,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");