database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-03 09:13:20 +03:00
Code Activity

Prevent race condition while reading relmapper file.

Browse Source 
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
This commit is contained in:
Heikki Linnakangas
2021-06-24 10:45:23 +03:00
parent 7a4ecefe9d
commit 2a0ab13f8d
1 changed files with 20 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
src/backend/utils/cache/relmapper.c vendored
View File

@@ -136,7 +136,7 @@ static void apply_map_update(RelMapFile *map, Oid relationId, Oid fileNode,
bool add_okay); bool add_okay);
static void merge_map_updates(RelMapFile *map, const RelMapFile *updates, static void merge_map_updates(RelMapFile *map, const RelMapFile *updates,
bool add_okay); 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, static void write_relmap_file(bool shared, RelMapFile *newmap,
bool write_wal, bool send_sinval, bool preserve_files, bool write_wal, bool send_sinval, bool preserve_files,
Oid dbid, Oid tsid, const char *dbpath); Oid dbid, Oid tsid, const char *dbpath);
@@ -405,12 +405,12 @@ RelationMapInvalidate(bool shared)
if (shared) if (shared)
{ {
if (shared_map.magic == RELMAPPER_FILEMAGIC) if (shared_map.magic == RELMAPPER_FILEMAGIC)
load_relmap_file(true); load_relmap_file(true, false);
} }
else else
{ {
if (local_map.magic == RELMAPPER_FILEMAGIC) if (local_map.magic == RELMAPPER_FILEMAGIC)
load_relmap_file(false); load_relmap_file(false, false);
} }
} }
@@ -425,9 +425,9 @@ void
RelationMapInvalidateAll(void) RelationMapInvalidateAll(void)
{ {
if (shared_map.magic == RELMAPPER_FILEMAGIC) if (shared_map.magic == RELMAPPER_FILEMAGIC)
load_relmap_file(true); load_relmap_file(true, false);
if (local_map.magic == RELMAPPER_FILEMAGIC) if (local_map.magic == RELMAPPER_FILEMAGIC)
load_relmap_file(false); load_relmap_file(false, false);
} }
/* /*
@@ -612,7 +612,7 @@ RelationMapInitializePhase2(void)
/* /*
* Load the shared map file, die on error. * Load the shared map file, die on error.
*/ */
load_relmap_file(true); load_relmap_file(true, false);
} }
/* /*
@@ -633,7 +633,7 @@ RelationMapInitializePhase3(void)
/* /*
* Load the local map file, die on error. * Load the local map file, die on error.
*/ */
load_relmap_file(false); load_relmap_file(false, false);
} }
/* /*
@@ -695,7 +695,7 @@ RestoreRelationMap(char *startAddress)
* Note that the local case requires DatabasePath to be set up. * Note that the local case requires DatabasePath to be set up.
*/ */
static void static void
load_relmap_file(bool shared) load_relmap_file(bool shared, bool lock_held)
{ {
RelMapFile *map; RelMapFile *map;
char mapfilename[MAXPGPATH]; char mapfilename[MAXPGPATH];
@@ -725,12 +725,15 @@ load_relmap_file(bool shared)
mapfilename))); mapfilename)));
/* /*
* Note: we could take RelationMappingLock in shared mode here, but it * Grab the lock to prevent the file from being updated while we read it,
* seems unnecessary since our read() should be atomic against any * unless the caller is already holding the lock. If the file is updated
* concurrent updater's write(). If the file is updated shortly after we * shortly after we look, the sinval signaling mechanism will make us
* look, the sinval signaling mechanism will make us re-read it before we * re-read it before we are able to access any relation that's affected by
* are able to access any relation that's affected by the change. * the change.
*/ */
if (!lock_held)
LWLockAcquire(RelationMappingLock, LW_SHARED);
pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ); pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ);
r = read(fd, map, sizeof(RelMapFile)); r = read(fd, map, sizeof(RelMapFile));
if (r != sizeof(RelMapFile)) if (r != sizeof(RelMapFile))
@@ -747,6 +750,9 @@ load_relmap_file(bool shared)
} }
pgstat_report_wait_end(); pgstat_report_wait_end();
if (!lock_held)
LWLockRelease(RelationMappingLock);
if (CloseTransientFile(fd) != 0) if (CloseTransientFile(fd) != 0)
ereport(FATAL, ereport(FATAL,
(errcode_for_file_access(), (errcode_for_file_access(),
@@ -969,7 +975,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates)
LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE); LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE);
/* Be certain we see any other updates just made */ /* 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 */ /* Prepare updated data in a local variable */
if (shared) if (shared)