Fix race condition in CheckTargetForConflictsIn.

Dan Ports

src/backend/storage/lmgr/predicate.c

@@ -3638,6 +3638,8 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
 	LWLockId	partitionLock;
 	PREDICATELOCKTARGET *target;
 	PREDICATELOCK *predlock;
+	PREDICATELOCK *mypredlock = NULL;
+	PREDICATELOCKTAG mypredlocktag;
 
 	Assert(MySerializableXact != InvalidSerializableXact);
 
@@ -3683,11 +3685,58 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
 		if (sxact == MySerializableXact)
 		{
 			/*
-			 * If we're getting a write lock on the tuple and we're not in a
-			 * subtransaction, we don't need a predicate (SIREAD) lock.  We
-			 * can't use this optimization within a subtransaction because the
-			 * subtransaction could be rolled back, and we would be left
-			 * without any lock at the top level.
+			 * If we're getting a write lock on a tuple, we don't need
+			 * a predicate (SIREAD) lock on the same tuple. We can
+			 * safely remove our SIREAD lock, but we'll defer doing so
+			 * until after the loop because that requires upgrading to
+			 * an exclusive partition lock.
+			 *
+			 * We can't use this optimization within a subtransaction
+			 * because the subtransaction could roll back, and we
+			 * would be left without any lock at the top level.
+			 */
+			if (!IsSubTransaction()
+				&& GET_PREDICATELOCKTARGETTAG_OFFSET(*targettag))
+			{
+				mypredlock = predlock;
+				mypredlocktag = predlock->tag;
+			}
+		}
+		else if (!SxactIsRolledBack(sxact)
+				 && (!SxactIsCommitted(sxact)
+					 || TransactionIdPrecedes(GetTransactionSnapshot()->xmin,
+											  sxact->finishedBefore))
+		&& !RWConflictExists(sxact, (SERIALIZABLEXACT *) MySerializableXact))
+		{
+			LWLockRelease(SerializableXactHashLock);
+			LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
+
+			/*
+			 * Re-check after getting exclusive lock because the other
+			 * transaction may have flagged a conflict.
+			 */
+			if (!SxactIsRolledBack(sxact)
+				&& (!SxactIsCommitted(sxact)
+					|| TransactionIdPrecedes(GetTransactionSnapshot()->xmin,
+											 sxact->finishedBefore))
+				&& !RWConflictExists(sxact,
+									 (SERIALIZABLEXACT *) MySerializableXact))
+			{
+				FlagRWConflict(sxact, (SERIALIZABLEXACT *) MySerializableXact);
+			}
+
+			LWLockRelease(SerializableXactHashLock);
+			LWLockAcquire(SerializableXactHashLock, LW_SHARED);
+		}
+
+		predlock = nextpredlock;
+	}
+	LWLockRelease(SerializableXactHashLock);
+	LWLockRelease(partitionLock);
+
+	/*
+	 * If we found one of our own SIREAD locks to remove, remove it
+	 * now.
 	 *
 	 * At this point our transaction already has an ExclusiveRowLock
 	 * on the relation, so we are OK to drop the predicate lock on the
@@ -3695,22 +3744,11 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
 	 * tuple will occur before the MVCC information makes it to the
 	 * buffer.
 	 */
-			if (!IsSubTransaction()
-				&& GET_PREDICATELOCKTARGETTAG_OFFSET(*targettag))
+	if (mypredlock != NULL)
 	{
 		uint32		predlockhashcode;
-				PREDICATELOCKTARGET *rmtarget = NULL;
 		PREDICATELOCK *rmpredlock;
-				LOCALPREDICATELOCK *locallock,
-						   *rmlocallock;
 
-				/*
-				 * This is a tuple on which we have a tuple predicate lock. We
-				 * only have shared LW locks now; release those, and get
-				 * exclusive locks only while we modify things.
-				 */
-				LWLockRelease(SerializableXactHashLock);
-				LWLockRelease(partitionLock);
 		LWLockAcquire(SerializablePredicateLockListLock, LW_SHARED);
 		LWLockAcquire(partitionLock, LW_EXCLUSIVE);
 		LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
@@ -3721,134 +3759,47 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag)
 		 * happen is from autovacuum cleaning up an index.
 		 */
 		predlockhashcode = PredicateLockHashCodeFromTargetHashCode
-					(&(predlock->tag), targettaghash);
+			(&mypredlocktag, targettaghash);
 		rmpredlock = (PREDICATELOCK *)
 			hash_search_with_hash_value(PredicateLockHash,
-												&(predlock->tag),
+										&mypredlocktag,
 										predlockhashcode,
 										HASH_FIND, NULL);
-				if (rmpredlock)
+		if (rmpredlock != NULL)
 		{
-					Assert(rmpredlock == predlock);
+			Assert(rmpredlock == mypredlock);
 
-					SHMQueueDelete(predlocktargetlink);
-					SHMQueueDelete(&(predlock->xactLink));
+			SHMQueueDelete(&(mypredlock->targetLink));
+			SHMQueueDelete(&(mypredlock->xactLink));
 
 			rmpredlock = (PREDICATELOCK *)
 				hash_search_with_hash_value(PredicateLockHash,
-													&(predlock->tag),
+											&mypredlocktag,
 											predlockhashcode,
 											HASH_REMOVE, NULL);
-					Assert(rmpredlock == predlock);
+			Assert(rmpredlock == mypredlock);
 
 			RemoveTargetIfNoLongerUsed(target, targettaghash);
+		}
 
 		LWLockRelease(SerializableXactHashLock);
 		LWLockRelease(partitionLock);
 		LWLockRelease(SerializablePredicateLockListLock);
 		
-					locallock = (LOCALPREDICATELOCK *)
-						hash_search_with_hash_value(LocalPredicateLockHash,
-													targettag, targettaghash,
-													HASH_FIND, NULL);
-
+		if (rmpredlock != NULL)
+		{
 			/*
-					 * Remove entry in local lock table if it exists and has
-					 * no children. It's OK if it doesn't exist; that means
-					 * the lock was transferred to a new target by a different
-					 * backend.
+			 * Remove entry in local lock table if it exists. It's OK
+			 * if it doesn't exist; that means the lock was
+			 * transferred to a new target by a different backend.
 			 */
-					if (locallock != NULL)
-					{
-						locallock->held = false;
-
-						if (locallock->childLocks == 0)
-						{
-							rmlocallock = (LOCALPREDICATELOCK *)
 			hash_search_with_hash_value(LocalPredicateLockHash,
 										targettag, targettaghash,
 										HASH_REMOVE, NULL);
-							Assert(rmlocallock == locallock);
-						}
-					}
 
 			DecrementParentLocks(targettag);
-
-					/*
-					 * If we've cleaned up the last of the predicate locks for
-					 * the target, bail out before re-acquiring the locks.
-					 */
-					if (rmtarget)
-						return;
-
-					/*
-					 * The list has been altered.  Start over at the front.
-					 */
-					LWLockAcquire(partitionLock, LW_SHARED);
-					nextpredlock = (PREDICATELOCK *)
-						SHMQueueNext(&(target->predicateLocks),
-									 &(target->predicateLocks),
-									 offsetof(PREDICATELOCK, targetLink));
-
-					LWLockAcquire(SerializableXactHashLock, LW_SHARED);
-				}
-				else
-				{
-					/*
-					 * The predicate lock was cleared while we were attempting
-					 * to upgrade our lightweight locks. Revert to the shared
-					 * locks.
-					 */
-					LWLockRelease(SerializableXactHashLock);
-					LWLockRelease(partitionLock);
-					LWLockRelease(SerializablePredicateLockListLock);
-					LWLockAcquire(partitionLock, LW_SHARED);
-
-					/*
-					 * The list may have been altered by another process while
-					 * we weren't holding the partition lock.  Start over at
-					 * the front.
-					 */
-					nextpredlock = (PREDICATELOCK *)
-						SHMQueueNext(&(target->predicateLocks),
-									 &(target->predicateLocks),
-									 offsetof(PREDICATELOCK, targetLink));
-
-					LWLockAcquire(SerializableXactHashLock, LW_SHARED);
-				}
 		}
 	}
-		else if (!SxactIsRolledBack(sxact)
-				 && (!SxactIsCommitted(sxact)
-					 || TransactionIdPrecedes(GetTransactionSnapshot()->xmin,
-											  sxact->finishedBefore))
-		&& !RWConflictExists(sxact, (SERIALIZABLEXACT *) MySerializableXact))
-		{
-			LWLockRelease(SerializableXactHashLock);
-			LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE);
-
-			/*
-			 * Re-check after getting exclusive lock because the other
-			 * transaction may have flagged a conflict.
-			 */
-			if (!SxactIsRolledBack(sxact)
-				&& (!SxactIsCommitted(sxact)
-					|| TransactionIdPrecedes(GetTransactionSnapshot()->xmin,
-											 sxact->finishedBefore))
-				&& !RWConflictExists(sxact,
-									 (SERIALIZABLEXACT *) MySerializableXact))
-			{
-				FlagRWConflict(sxact, (SERIALIZABLEXACT *) MySerializableXact);
-			}
-
-			LWLockRelease(SerializableXactHashLock);
-			LWLockAcquire(SerializableXactHashLock, LW_SHARED);
-		}
-
-		predlock = nextpredlock;
-	}
-	LWLockRelease(SerializableXactHashLock);
-	LWLockRelease(partitionLock);
 }
 
 /*