Fix race condition in relcache init file invalidation.

The previous code tried to synchronize by unlinking the init file twice,
but that doesn't actually work: it leaves a window wherein a third process
could read the already-stale init file but miss the SI messages that would
tell it the data is stale.  The result would be bizarre failures in catalog
accesses, typically "could not read block 0 in file ..." later during
startup.

Instead, hold RelCacheInitLock across both the unlink and the sending of
the SI messages.  This is more straightforward, and might even be a bit
faster since only one unlink call is needed.

This has been wrong since it was put in (in 2002!), so back-patch to all
supported releases.

1 files changed, 17 insertions, 16 deletions

diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index a5580bd92fb..4249bd33765 100644
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -854,24 +854,12 @@ xactGetCommittedInvalidationMessages(SharedInvalidationMessage **msgs,
 	return numSharedInvalidMessagesArray;
 }
 
-#define RecoveryRelationCacheInitFileInvalidate(dbo, tbo, tf) \
-{ \
-	DatabasePath = GetDatabasePath(dbo, tbo); \
-	elog(trace_recovery(DEBUG4), "removing relcache init file in %s", DatabasePath); \
-	RelationCacheInitFileInvalidate(tf); \
-	pfree(DatabasePath); \
-}
-
 /*
  * ProcessCommittedInvalidationMessages is executed by xact_redo_commit()
  * to process invalidation messages added to commit records.
  *
  * Relcache init file invalidation requires processing both
  * before and after we send the SI messages. See AtEOXact_Inval()
- *
- * We deliberately avoid SetDatabasePath() since it is intended to be used
- * only once by normal backends, so we set DatabasePath directly then
- * pfree after use. See RecoveryRelationCacheInitFileInvalidate() macro.
  */
 void
 ProcessCommittedInvalidationMessages(SharedInvalidationMessage *msgs,
@@ -885,12 +873,25 @@ ProcessCommittedInvalidationMessages(SharedInvalidationMessage *msgs,
 		 (RelcacheInitFileInval ? " and relcache file invalidation" : ""));
 
 	if (RelcacheInitFileInval)
-		RecoveryRelationCacheInitFileInvalidate(dbid, tsid, true);
+	{
+		/*
+		 * RelationCacheInitFilePreInvalidate requires DatabasePath to be set,
+		 * but we should not use SetDatabasePath during recovery, since it is
+		 * intended to be used only once by normal backends.  Hence, a quick
+		 * hack: set DatabasePath directly then unset after use.
+		 */
+		DatabasePath = GetDatabasePath(dbid, tsid);
+		elog(trace_recovery(DEBUG4), "removing relcache init file in \"%s\"",
+			 DatabasePath);
+		RelationCacheInitFilePreInvalidate();
+		pfree(DatabasePath);
+		DatabasePath = NULL;
+	}
 
 	SendSharedInvalidMessages(msgs, nmsgs);
 
 	if (RelcacheInitFileInval)
-		RecoveryRelationCacheInitFileInvalidate(dbid, tsid, false);
+		RelationCacheInitFilePostInvalidate();
 }
 
 /*
@@ -931,7 +932,7 @@ AtEOXact_Inval(bool isCommit)
 		 * unless we committed.
 		 */
 		if (transInvalInfo->RelcacheInitFileInval)
-			RelationCacheInitFileInvalidate(true);
+			RelationCacheInitFilePreInvalidate();
 
 		AppendInvalidationMessages(&transInvalInfo->PriorCmdInvalidMsgs,
 								   &transInvalInfo->CurrentCmdInvalidMsgs);
@@ -940,7 +941,7 @@ AtEOXact_Inval(bool isCommit)
 										 SendSharedInvalidMessages);
 
 		if (transInvalInfo->RelcacheInitFileInval)
-			RelationCacheInitFileInvalidate(false);
+			RelationCacheInitFilePostInvalidate();
 	}
 	else if (transInvalInfo != NULL)
 	{