kolan
/
CMake
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix cmSystemTools::RenameFile race on Windows 

Since commit d46d8df0 (Re-implemented cmGeneratedFileStream to look like a
real stream and replace the destination file atomically, 2004-11-03) our
RenameFile implementation tries to deal with MoveFile not replacing
read-only files.  In order to avoid the Get/SetFileAttributes pair we used
stat to test for existence of the destination file.  This has a race in
which the destination could be created between the test for existence and
the MoveFile call.

Remove the stat call and use GetFileAttributes to detect whether the file
exists.  This shortens the race but does not eliminate it.  Use a loop to
try multiple times in case we lose the race.  While at it, drop Win9x
support and always use MoveFileEx.
This commit is contained in:
Brad King 2013-01-31 11:42:17 -05:00
parent 21fc6c46df
commit 59b568e5e8
1 changed files with 20 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
Source/cmSystemTools.cxx
View File

@ -1180,46 +1180,35 @@ bool cmSystemTools::CopyFileIfDifferent(const char* source,
bool cmSystemTools::RenameFile(const char* oldname, const char* newname) bool cmSystemTools::RenameFile(const char* oldname, const char* newname)
{ {
#ifdef _WIN32 #ifdef _WIN32
/* On Windows the move functions will not replace existing files. # ifndef INVALID_FILE_ATTRIBUTES
Check if the destination exists. */ # define INVALID_FILE_ATTRIBUTES ((DWORD)-1)
struct stat newFile; # endif
if(stat(newname, &newFile) == 0) /* Windows MoveFileEx may not replace read-only or in-use files. If it
fails then remove the read-only attribute from any existing destination.
Try multiple times since we may be racing against another process
creating/opening the destination file just before our MoveFileEx. */
int tries = 5;
while(!MoveFileEx(oldname, newname, MOVEFILE_REPLACE_EXISTING) && --tries)
{ {
/* The destination exists. We have to replace it carefully. The // Try again only if failure was due to access permissions.
MoveFileEx function does what we need but is not available on if(GetLastError() != ERROR_ACCESS_DENIED)
Win9x. */
OSVERSIONINFO osv;
DWORD attrs;
/* Make sure the destination is not read only. */
attrs = GetFileAttributes(newname);
if(attrs & FILE_ATTRIBUTE_READONLY)
{ {
SetFileAttributes(newname, attrs & ~FILE_ATTRIBUTE_READONLY); return false;
} }
DWORD attrs = GetFileAttributes(newname);
/* Check the windows version number. */ if((attrs != INVALID_FILE_ATTRIBUTES) &&
osv.dwOSVersionInfoSize = sizeof(osv); (attrs & FILE_ATTRIBUTE_READONLY))
GetVersionEx(&osv);
if(osv.dwPlatformId == VER_PLATFORM_WIN32_WINDOWS)
{ {
/* This is Win9x. There is no MoveFileEx implementation. We // Remove the read-only attribute from the destination file.
cannot quite rename the file atomically. Just delete the SetFileAttributes(newname, attrs & ~FILE_ATTRIBUTE_READONLY);
destination and then move the file. */
DeleteFile(newname);
return MoveFile(oldname, newname) != 0;
} }
else else
{ {
/* This is not Win9x. Use the MoveFileEx implementation. */ // The file may be temporarily in use so wait a bit.
return MoveFileEx(oldname, newname, MOVEFILE_REPLACE_EXISTING) != 0; cmSystemTools::Delay(100);
} }
} }
else return tries > 0;
{
/* The destination does not exist. Just move the file. */
return MoveFile(oldname, newname) != 0;
}
#else #else
/* On UNIX we have an OS-provided call to do this atomically. */ /* On UNIX we have an OS-provided call to do this atomically. */
return rename(oldname, newname) == 0; return rename(oldname, newname) == 0;