Author: kotkov
Date: Tue Aug 20 08:48:32 2019
New Revision: 1865518
URL: http://svn.apache.org/viewvc?rev=1865518&view=rev
Log:
Win32: prevent svn_io_file_rename2() from spinning in the retry loop in a
racy case where the rename function clears the read-only attribute from the
destination file, but another thread or process is quick enough to set it
back before the original rename function has a chance to proceed.
Despite sounding like an uncommon case, this may happen when the API is
being used to atomically update the file contents while also altering its
read-only attribute (and where such updates can occur in parallel).
Fix this by including the call that clears the read-only attribute on the
destination into the retry loop instead of calling it only once before
looping.
* subversion/libsvn_subr/io.c
(win32_file_rename): Handle EACCES/EEXIST and attempt to clear the
destination read-only attribute if it's there.
(svn_io_file_rename2): Don't call svn_io_set_file_read_write() in the
Win32-specific portion of this function, since clearing the read-only
attribute is now a part of the win32_file_rename() function.
Modified:
subversion/trunk/subversion/libsvn_subr/io.c
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1865518&r1=1865517&r2=1865518&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Tue Aug 20 08:48:32 2019
@@ -4491,7 +4491,43 @@ win32_file_rename(const WCHAR *from_path
}
if (!MoveFileExW(from_path_w, to_path_w, flags))
- return apr_get_os_error();
+ {
+ apr_status_t status = apr_get_os_error();
+ /* If the target file is read only NTFS reports EACCESS and
+ FAT/FAT32 reports EEXIST */
+ if (APR_STATUS_IS_EACCES(status) || APR_STATUS_IS_EEXIST(status))
+ {
+ DWORD attrs = GetFileAttributesW(to_path_w);
+ if (attrs == INVALID_FILE_ATTRIBUTES)
+ {
+ status = apr_get_os_error();
+ if (!(APR_STATUS_IS_ENOENT(status) ||
SVN__APR_STATUS_IS_ENOTDIR(status)))
+ return status;
+ }
+ else if (attrs & FILE_ATTRIBUTE_READONLY)
+ {
+ /* Try to set the destination file writable because Windows will
+ not allow us to rename when to_path is read-only, but will
+ allow renaming when from_path is read only. */
+ attrs &= ~FILE_ATTRIBUTE_READONLY;
+ if (!SetFileAttributesW(to_path_w, attrs))
+ {
+ status = apr_get_os_error();
+ if (!(APR_STATUS_IS_ENOENT(status) ||
SVN__APR_STATUS_IS_ENOTDIR(status)))
+ return status;
+ }
+ }
+
+ /* NOTE: If the file is not read-only, we don't know if the file did
+ not have the read-only attribute in the first place or if this
+ attribute disappeared due to a race, so try to rename it anyway.
+ */
+ if (!MoveFileExW(from_path_w, to_path_w, flags))
+ return apr_get_os_error();
+ }
+ else
+ return status;
+ }
return APR_SUCCESS;
}
@@ -4515,18 +4551,6 @@ svn_io_file_rename2(const char *from_pat
SVN_ERR(svn_io__utf8_to_unicode_longpath(&from_path_w, from_path_apr, pool));
SVN_ERR(svn_io__utf8_to_unicode_longpath(&to_path_w, to_path_apr, pool));
status = win32_file_rename(from_path_w, to_path_w, flush_to_disk);
-
- /* If the target file is read only NTFS reports EACCESS and
- FAT/FAT32 reports EEXIST */
- if (APR_STATUS_IS_EACCES(status) || APR_STATUS_IS_EEXIST(status))
- {
- /* Set the destination file writable because Windows will not
- allow us to rename when to_path is read-only, but will
- allow renaming when from_path is read only. */
- SVN_ERR(svn_io_set_file_read_write(to_path, TRUE, pool));
-
- status = win32_file_rename(from_path_w, to_path_w, flush_to_disk);
- }
WIN32_RETRY_LOOP(status, win32_file_rename(from_path_w, to_path_w,
flush_to_disk));
#elif defined(__OS2__)
