mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-10-06 08:57:02 +02:00
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
This commit is contained in:
parent
e00e5db22d
commit
c78bb32c19
34
src/backend/utils/cache/relmapper.c
vendored
34
src/backend/utils/cache/relmapper.c
vendored
@ -123,7 +123,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);
|
||||||
@ -393,12 +393,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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -413,9 +413,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);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -594,7 +594,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);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -615,7 +615,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);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -627,7 +627,7 @@ RelationMapInitializePhase3(void)
|
|||||||
* 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];
|
||||||
@ -656,12 +656,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);
|
||||||
if (read(fd, map, sizeof(RelMapFile)) != sizeof(RelMapFile))
|
if (read(fd, map, sizeof(RelMapFile)) != sizeof(RelMapFile))
|
||||||
ereport(FATAL,
|
ereport(FATAL,
|
||||||
@ -670,6 +673,9 @@ load_relmap_file(bool shared)
|
|||||||
mapfilename)));
|
mapfilename)));
|
||||||
pgstat_report_wait_end();
|
pgstat_report_wait_end();
|
||||||
|
|
||||||
|
if (!lock_held)
|
||||||
|
LWLockRelease(RelationMappingLock);
|
||||||
|
|
||||||
CloseTransientFile(fd);
|
CloseTransientFile(fd);
|
||||||
|
|
||||||
/* check for correct magic number, etc */
|
/* check for correct magic number, etc */
|
||||||
@ -888,7 +894,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)
|
||||||
|
Loading…
Reference in New Issue
Block a user