8000 Prevent race condition while reading relmapper file. · postgrespro/postgres@c78bb32 · GitHub
[go: up one dir, main page]
Skip to content

Commit c78bb32

Browse files
hlinnakahlinnaka
committed
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
1 parent e00e5db commit c78bb32

File tree

1 file changed

+20
-14
lines changed

1 file changed

+20
-14
lines changed

‎src/backend/utils/cache/relmapper.c

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ static void apply_map_update(RelMapFile *map, Oid relationId, Oid fileNode,
123123
bool add_okay);
124124
static void merge_map_updates(RelMapFile *map, const RelMapFile *updates,
125125
bool add_okay);
126-
static void load_relmap_file(bool shared);
126+
static void load_relmap_file(bool shared, bool lock_held);
127127
static void write_relmap_file(bool shared, RelMapFile *newmap,
128128
bool write_wal, bool send_sinval, bool preserve_files,
129129
Oid dbid, Oid tsid, const char *dbpath);
@@ -393,12 +393,12 @@ RelationMapInvalidate(bool shared)
393393
if (shared)
394394
{
395395
if (shared_map.magic == RELMAPPER_FILEMAGIC)
396-
load_relmap_file(true);
396+
load_relmap_file(true, false);
397397
}
398398
else
399399
{
400400
if (local_map.magic == RELMAPPER_FILEMAGIC)
401-
load_relmap_file(false);
401+
load_relmap_file(false, false);
402402
}
403403
}
404404

@@ -413,9 +413,9 @@ void
413413
RelationMapInvalidateAll(void)
414414
{
415415
if (shared_map.magic == RELMAPPER_FILEMAGIC)
416-
load_relmap_file(true);
416+
load_relmap_file(true, false);
417417
if (local_map.magic == RELMAPPER_FILEMAGIC)
418-
load_relmap_file(false);
418+
load_ 8000 relmap_file(false, false);
419419
}
420420

421421
/*
@@ -594,7 +594,7 @@ RelationMapInitializePhase2(void)
594594
/*
595595
* Load the shared map file, die on error.
596596
*/
597-
load_relmap_file(true);
597+
load_relmap_file(true, false);
598598
}
599599

600600
/*
@@ -615,7 +615,7 @@ RelationMapInitializePhase3(void)
615615
/*
616616
* Load the local map file, die on error.
617617
*/
618-
load_relmap_file(false);
618+
load_relmap_file(false, false);
619619
}
620620

621621
/*
@@ -627,7 +627,7 @@ RelationMapInitializePhase3(void)
627627
* Note that the local case requires DatabasePath to be set up.
628628
*/
629629
static void
630-
load_relmap_file(bool shared)
630+
load_relmap_file(bool shared, bool lock_held)
631631
{
632632
RelMapFile *map;
633633
char mapfilename[MAXPGPATH];
@@ -656,12 +656,15 @@ load_relmap_file(bool shared)
656656
mapfilename)));
657657

658658
/*
659-
* Note: we could take RelationMappingLock in shared mode here, but it
660-
* seems unnecessary since our read() should be atomic against any
661-
* concurrent updater's write(). If the file is updated shortly after we
662-
* look, the sinval signaling mechanism will make us re-read it before we
663-
* are able to access any relation that's affected by the change.
659+
* Grab the lock to prevent the file from being updated while we read it,
660+
* unless the caller is already holding the lock. If the file is updated
661+
* shortly after we look, the sinval signaling mechanism will make us
662+
* re-read it before we are able to access any relation that's affected by
663+
* the change.
664664
*/
665+
if (!lock_held)
666+
LWLockAcquire(RelationMappingLock, LW_SHARED);
667+
665668
pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ);
666669
if (read(fd, map, sizeof(RelMapFile)) != sizeof(RelMapFile))
667670
ereport(FATAL,
@@ -670,6 +673,9 @@ load_relmap_file(bool shared)
670673
mapfilename)));
671674
pgstat_report_wait_end();
672675

676+
if (!lock_held)
677+
LWLockRelease(RelationMappingLock);
678+
673679
CloseTransientFile(fd);
674680

675681
/* check for correct magic number, etc */
@@ -888,7 +894,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates)
888894
LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE);
889895

890896
/* Be certain we see any other updates just made */
891-
load_relmap_file(shared);
897+
load_relmap_file(shared, true);
892898

893899
/* Prepare updated data in a local variable */
894900
if (shared)

0 commit comments

Comments
 (0)
0