Move call to GetTopTransactionId() earlier in LockAcquire(),

removing an infrequently occurring race condition in Hot Standby.
An xid must be assigned before a lock appears in shared memory,
rather than immediately after, else GetRunningTransactionLocks()
may see InvalidTransactionId, causing assertion failures during
lock processing on standby.

Bug report and diagnosis by Fujii Masao, fix by me.

1 files changed, 21 insertions, 8 deletions

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 154147e44c2..ca094a01b40 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -953,14 +953,6 @@ LogAccessExclusiveLock(Oid dbOid, Oid relOid)
 {
 	xl_standby_lock xlrec;
 
-	/*
-	 * Ensure that a TransactionId has been assigned to this transaction. We
-	 * don't actually need the xid yet but if we don't do this then
-	 * RecordTransactionCommit() and RecordTransactionAbort() will optimise
-	 * away the transaction completion record which recovery relies upon to
-	 * release locks. It's a hack, but for a corner case not worth adding code
-	 * for into the main commit path.
-	 */
 	xlrec.xid = GetTopTransactionId();
 
 	/*
@@ -973,3 +965,24 @@ LogAccessExclusiveLock(Oid dbOid, Oid relOid)
 
 	LogAccessExclusiveLocks(1, &xlrec);
 }
+
+/*
+ * Prepare to log an AccessExclusiveLock, for use during LockAcquire()
+ */
+void
+LogAccessExclusiveLockPrepare(void)
+{
+	/*
+	 * Ensure that a TransactionId has been assigned to this transaction,
+	 * for two reasons, both related to lock release on the standby.
+	 * First, we must assign an xid so that RecordTransactionCommit() and
+	 * RecordTransactionAbort() do not optimise away the transaction
+	 * completion record which recovery relies upon to release locks. It's
+	 * a hack, but for a corner case not worth adding code for into the
+	 * main commit path. Second, must must assign an xid before the lock
+	 * is recorded in shared memory, otherwise a concurrently executing
+	 * GetRunningTransactionLocks() might see a lock associated with an
+	 * InvalidTransactionId which we later assert cannot happen.
+	 */
+	(void) GetTopTransactionId();
+}