commit 80b3a109b5af25190343d3307eeeb6584b77b8b3 Author: Alexander Korotkov Date: Tue Aug 6 20:49:03 2019 +0300 Introduce pgrename_temp() Reported-by: Bug: Discussion: Author: Reviewed-by: Tested-by: Backpatch-through: diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 2bb14cdd026..b4404f23805 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -4952,7 +4952,7 @@ pgstat_write_statsfiles(bool permanent, bool allDbs) tmpfile))); unlink(tmpfile); } - else if (rename(tmpfile, statfile) < 0) + else if (rename_temp(tmpfile, statfile) < 0) { ereport(LOG, (errcode_for_file_access(), @@ -5087,7 +5087,7 @@ pgstat_write_db_statsfile(PgStat_StatDBEntry *dbentry, bool permanent) tmpfile))); unlink(tmpfile); } - else if (rename(tmpfile, statfile) < 0) + else if (rename_temp(tmpfile, statfile) < 0) { ereport(LOG, (errcode_for_file_access(), diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index bafd31d22bb..734cefebf02 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -1518,7 +1518,7 @@ update_metainfo_datafile(void) } fclose(fh); - if (rename(LOG_METAINFO_DATAFILE_TMP, LOG_METAINFO_DATAFILE) != 0) + if (rename_temp(LOG_METAINFO_DATAFILE_TMP, LOG_METAINFO_DATAFILE) != 0) ereport(LOG, (errcode_for_file_access(), errmsg("could not rename file \"%s\" to \"%s\": %m", diff --git a/src/include/port.h b/src/include/port.h index b5c03d912b0..fb6446ade5e 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -234,14 +234,28 @@ extern int pclose_check(FILE *stream); extern int pgrename(const char *from, const char *to); extern int pgunlink(const char *path); +#ifdef WIN32 +extern int pgrename_temp(const char *from, const char *to); +#endif + /* Include this first so later includes don't see these defines */ #ifdef _MSC_VER #include #endif +#ifdef WIN32 +#define rename_temp(from, to) pgrename_temp(from, to) +#else +#define rename_temp(from, to) pgrename(from, to) +#endif + #define rename(from, to) pgrename(from, to) #define unlink(path) pgunlink(path) -#endif /* defined(WIN32) || defined(__CYGWIN__) */ +#else /* defined(WIN32) || defined(__CYGWIN__) */ + +#define rename_temp(from, to) rename(from, to) + +#endif /* * Win32 also doesn't have symlinks, but we can emulate them with diff --git a/src/port/dirmod.c b/src/port/dirmod.c index d7932401ef0..fa7286b9720 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -92,6 +92,87 @@ pgrename(const char *from, const char *to) } +#ifdef WIN32 + +/* + * pgrename_temp + * + * Renames file concurrently to operations on target. Not safe version, if OS + * crashes during replacing a file, then afterwards target filename might not + * exists. Suitable for temporary files. + */ +int +pgrename_temp(const char *from, const char *to) +{ + int loops = 0; + + /* + * We need to loop because even though PostgreSQL uses flags that allow + * rename while the file is open, other applications might have the file + * open without those flags. However, we won't wait indefinitely for + * someone else to close the file, as the caller might be holding locks + * and blocking other backends. + */ + DWORD err; + + /* + * On Windows we use ReplaceFile() to rename while concurrent processes + * have file open. However, ReplaceFile() is to be used only when target + * file is already exists. Thus, we check for file existence and then + * choose between MoveFileEx() and ReplaceFile() functions. + */ + while (true) + { + DWORD dwAttrib; + bool filePresent; + + dwAttrib = GetFileAttributes(to); + filePresent = (dwAttrib != INVALID_FILE_ATTRIBUTES) && + !(dwAttrib & FILE_ATTRIBUTE_DIRECTORY); + + if (filePresent) + { + if (ReplaceFile(to, from, NULL, REPLACEFILE_IGNORE_MERGE_ERRORS, 0, 0)) + break; + } + else + { + if (MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING)) + break; + } + err = GetLastError(); + + _dosmaperr(err); + + /* + * Modern NT-based Windows versions return ERROR_SHARING_VIOLATION if + * another process has the file open without FILE_SHARE_DELETE. + * ERROR_LOCK_VIOLATION has also been seen with some anti-virus + * software. This used to check for just ERROR_ACCESS_DENIED, so + * presumably you can get that too with some OS versions. We don't + * expect real permission errors where we currently use rename(). + * + * Due to cuncurrent operation ReplaceFile() might return either + * ERROR_UNABLE_TO_MOVE_REPLACEMENT or + * ERROR_UNABLE_TO_REMOVE_REPLACED. In both cases it worth retrying. + */ + if (err != ERROR_ACCESS_DENIED && + err != ERROR_SHARING_VIOLATION && + err != ERROR_LOCK_VIOLATION && + err != ERROR_UNABLE_TO_MOVE_REPLACEMENT && + err != ERROR_UNABLE_TO_REMOVE_REPLACED) + return -1; + + if (++loops > 100) /* time out after 10 sec */ + return -1; + pg_usleep(100000); /* us */ + } + return 0; +} + +#endif + + /* * pgunlink */