Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 85ae8cc

Browse filesBrowse files
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 4be39ef commit 85ae8cc
Copy full SHA for 85ae8cc

File tree

1 file changed

+20
-14
lines changed
Filter options

1 file changed

+20
-14
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/utils/cache/relmapper.c
+20-14Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ static void apply_map_update(RelMapFile *map, Oid relationId, Oid fileNode,
122122
bool add_okay);
123123
static void merge_map_updates(RelMapFile *map, const RelMapFile *updates,
124124
bool add_okay);
125-
static void load_relmap_file(bool shared);
125+
static void load_relmap_file(bool shared, bool lock_held);
126126
static void write_relmap_file(bool shared, RelMapFile *newmap,
127127
bool write_wal, bool send_sinval, bool preserve_files,
128128
Oid dbid, Oid tsid, const char *dbpath);
@@ -388,12 +388,12 @@ RelationMapInvalidate(bool shared)
388388
if (shared)
389389
{
390390
if (shared_map.magic == RELMAPPER_FILEMAGIC)
391-
load_relmap_file(true);
391+
load_relmap_file(true, false);
392392
}
393393
else
394394
{
395395
if (local_map.magic == RELMAPPER_FILEMAGIC)
396-
load_relmap_file(false);
396+
load_relmap_file(false, false);
397397
}
398398
}
399399

@@ -408,9 +408,9 @@ void
408408
RelationMapInvalidateAll(void)
409409
{
410410
if (shared_map.magic == RELMAPPER_FILEMAGIC)
411-
load_relmap_file(true);
411+
load_relmap_file(true, false);
412412
if (local_map.magic == RELMAPPER_FILEMAGIC)
413-
load_relmap_file(false);
413+
load_relmap_file(false, false);
414414
}
415415

416416
/*
@@ -589,7 +589,7 @@ RelationMapInitializePhase2(void)
589589
/*
590590
* Load the shared map file, die on error.
591591
*/
592-
load_relmap_file(true);
592+
load_relmap_file(true, false);
593593
}
594594

595595
/*
@@ -610,7 +610,7 @@ RelationMapInitializePhase3(void)
610610
/*
611611
* Load the local map file, die on error.
612612
*/
613-
load_relmap_file(false);
613+
load_relmap_file(false, false);
614614
}
615615

616616
/*
@@ -622,7 +622,7 @@ RelationMapInitializePhase3(void)
622622
* Note that the local case requires DatabasePath to be set up.
623623
*/
624624
static void
625-
load_relmap_file(bool shared)
625+
load_relmap_file(bool shared, bool lock_held)
626626
{
627627
RelMapFile *map;
628628
char mapfilename[MAXPGPATH];
@@ -652,18 +652,24 @@ load_relmap_file(bool shared)
652652
mapfilename)));
653653

654654
/*
655-
* Note: we could take RelationMappingLock in shared mode here, but it
656-
* seems unnecessary since our read() should be atomic against any
657-
* concurrent updater's write(). If the file is updated shortly after we
658-
* look, the sinval signaling mechanism will make us re-read it before we
659-
* are able to access any relation that's affected by the change.
655+
* Grab the lock to prevent the file from being updated while we read it,
656+
* unless the caller is already holding the lock. If the file is updated
657+
* shortly after we look, the sinval signaling mechanism will make us
658+
* re-read it before we are able to access any relation that's affected by
659+
* the change.
660660
*/
661+
if (!lock_held)
662+
LWLockAcquire(RelationMappingLock, LW_SHARED);
663+
661664
if (read(fd, map, sizeof(RelMapFile)) != sizeof(RelMapFile))
662665
ereport(FATAL,
663666
(errcode_for_file_access(),
664667
errmsg("could not read relation mapping file \"%s\": %m",
665668
mapfilename)));
666669

670+
if (!lock_held)
671+
LWLockRelease(RelationMappingLock);
672+
667673
CloseTransientFile(fd);
668674

669675
/* check for correct magic number, etc */
@@ -880,7 +886,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates)
880886
LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE);
881887

882888
/* Be certain we see any other updates just made */
883-
load_relmap_file(shared);
889+
load_relmap_file(shared, true);
884890

885891
/* Prepare updated data in a local variable */
886892
if (shared)

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.