Fix read_relmap_file() concurrency on Windows.

Commit d8cd0c6c95 introduced a file
rename that could fail on Windows, probably due to other backends
having an open file handle to the old file of the same name.
Re-arrange the locking slightly to prevent that, by making sure the
open() and close() run while we hold the lock.

Thomas Munro. I added an explanatory comment.

Discussion: https://postgr.es/m/CA%2BhUKGLZtCTgp4NTWV-wGbR2Nyag71%3DEfYTKjDKnk%2BfkhuFMHw%40mail.gmail.com
This commit is contained in:
Robert Haas 2022-07-27 11:12:15 -04:00
parent ce3049b021
commit a2e97cb2b6

View File

@ -788,16 +788,6 @@ read_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, int elevel)
Assert(elevel >= ERROR);
/* Open the target file. */
snprintf(mapfilename, sizeof(mapfilename), "%s/%s", dbpath,
RELMAPPER_FILENAME);
fd = OpenTransientFile(mapfilename, O_RDONLY | PG_BINARY);
if (fd < 0)
ereport(elevel,
(errcode_for_file_access(),
errmsg("could not open file \"%s\": %m",
mapfilename)));
/*
* 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
@ -808,6 +798,24 @@ read_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, int elevel)
if (!lock_held)
LWLockAcquire(RelationMappingLock, LW_SHARED);
/*
* Open the target file.
*
* Because Windows isn't happy about the idea of renaming over a file
* that someone has open, we only open this file after acquiring the lock,
* and for the same reason, we close it before releasing the lock. That
* way, by the time write_relmap_file() acquires an exclusive lock, no
* one else will have it open.
*/
snprintf(mapfilename, sizeof(mapfilename), "%s/%s", dbpath,
RELMAPPER_FILENAME);
fd = OpenTransientFile(mapfilename, O_RDONLY | PG_BINARY);
if (fd < 0)
ereport(elevel,
(errcode_for_file_access(),
errmsg("could not open file \"%s\": %m",
mapfilename)));
/* Now read the data. */
pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ);
r = read(fd, map, sizeof(RelMapFile));
@ -825,15 +833,15 @@ read_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, int elevel)
}
pgstat_report_wait_end();
if (!lock_held)
LWLockRelease(RelationMappingLock);
if (CloseTransientFile(fd) != 0)
ereport(elevel,
(errcode_for_file_access(),
errmsg("could not close file \"%s\": %m",
mapfilename)));
if (!lock_held)
LWLockRelease(RelationMappingLock);
/* check for correct magic number, etc */
if (map->magic != RELMAPPER_FILEMAGIC ||
map->num_mappings < 0 ||