postgres.git / summary / log / commit / refs

commit    a5eacd86b71542d5e377d1fef628866dae859809
Author:   Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date:     Thu Jun 24 07:45:23 2021 +0000
First released in: 14.0, 9.6.23, 10.18, 11.13, 12.8, 13.4

    Prevent race condition while reading relmapper file.
    
    Contrary to the comment here, POSIX does not guarantee atomicity of a
    read(), if another process calls write() concurrently. Or at least Linux
    does not. Add locking to load_relmap_file() to avoid the race condition.
    
    Fixes bug #17064. Thanks to Alexander Lakhin for the report and test case.
    
    Backpatch-through: 9.6, all supported versions.
    Discussion: https://www.postgresql.org/message-id/17064-bb0d7904ef72add3@postgresql.org


 src/backend/utils/cache/relmapper.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c
index e4d59774850..e837c409e4c 100644
--- a/src/backend/utils/cache/relmapper.c
+++ b/src/backend/utils/cache/relmapper.c
@@ -123,7 +123,7 @@ static void apply_map_update(RelMapFile *map, Oid relationId, Oid fileNode,
 				 bool add_okay);
 static void merge_map_updates(RelMapFile *map, const RelMapFile *updates,
 				  bool add_okay);
-static void load_relmap_file(bool shared);
+static void load_relmap_file(bool shared, bool lock_held);
 static void write_relmap_file(bool shared, RelMapFile *newmap,
 				  bool write_wal, bool send_sinval, bool preserve_files,
 				  Oid dbid, Oid tsid, const char *dbpath);
@@ -389,12 +389,12 @@ RelationMapInvalidate(bool shared)
 	if (shared)
 	{
 		if (shared_map.magic == RELMAPPER_FILEMAGIC)
-			load_relmap_file(true);
+			load_relmap_file(true, false);
 	}
 	else
 	{
 		if (local_map.magic == RELMAPPER_FILEMAGIC)
-			load_relmap_file(false);
+			load_relmap_file(false, false);
 	}
 }
 
@@ -409,9 +409,9 @@ void
 RelationMapInvalidateAll(void)
 {
 	if (shared_map.magic == RELMAPPER_FILEMAGIC)
-		load_relmap_file(true);
+		load_relmap_file(true, false);
 	if (local_map.magic == RELMAPPER_FILEMAGIC)
-		load_relmap_file(false);
+		load_relmap_file(false, false);
 }
 
 /*
@@ -590,7 +590,7 @@ RelationMapInitializePhase2(void)
 	/*
 	 * Load the shared map file, die on error.
 	 */
-	load_relmap_file(true);
+	load_relmap_file(true, false);
 }
 
 /*
@@ -611,7 +611,7 @@ RelationMapInitializePhase3(void)
 	/*
 	 * Load the local map file, die on error.
 	 */
-	load_relmap_file(false);
+	load_relmap_file(false, false);
 }
 
 /*
@@ -623,7 +623,7 @@ RelationMapInitializePhase3(void)
  * Note that the local case requires DatabasePath to be set up.
  */
 static void
-load_relmap_file(bool shared)
+load_relmap_file(bool shared, bool lock_held)
 {
 	RelMapFile *map;
 	char		mapfilename[MAXPGPATH];
@@ -653,12 +653,15 @@ load_relmap_file(bool shared)
 						mapfilename)));
 
 	/*
-	 * Note: we could take RelationMappingLock in shared mode here, but it
-	 * seems unnecessary since our read() should be atomic against any
-	 * concurrent updater's write().  If the file is updated shortly after we
-	 * look, the sinval signaling mechanism will make us re-read it before we
-	 * are able to access any relation that's affected by the change.
+	 * Grab the lock to prevent the file from being updated while we read it,
+	 * unless the caller is already holding the lock.  If the file is updated
+	 * shortly after we look, the sinval signaling mechanism will make us
+	 * re-read it before we are able to access any relation that's affected by
+	 * the change.
 	 */
+	if (!lock_held)
+		LWLockAcquire(RelationMappingLock, LW_SHARED);
+
 	pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ);
 	if (read(fd, map, sizeof(RelMapFile)) != sizeof(RelMapFile))
 		ereport(FATAL,
@@ -667,6 +670,9 @@ load_relmap_file(bool shared)
 						mapfilename)));
 	pgstat_report_wait_end();
 
+	if (!lock_held)
+		LWLockRelease(RelationMappingLock);
+
 	CloseTransientFile(fd);
 
 	/* check for correct magic number, etc */
@@ -887,7 +893,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates)
 	LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE);
 
 	/* Be certain we see any other updates just made */
-	load_relmap_file(shared);
+	load_relmap_file(shared, true);
 
 	/* Prepare updated data in a local variable */
 	if (shared)


[parent: f365151a5392]