ExternalData: Avoid replacing a concurrently-created object

If more than one content link references the same object, the build
system may launch multiple download processes for the same object
concurrently.  Use whichever one finishes first, and discard the others.

Without this, we replace the objects and use the last finisher instead
of the first.  This is okay on non-Windows platforms where `rename(2)`
gives reliable atomic replacement.  However, on Windows platforms and
NTFS this is less reliable.  I've observed `MoveFileEx` somehow cause
another process to get `ERROR_SHARING_VIOLATION` when attempting to read
the destination file.  We may be able to improve the `file(RENAME)`
implementation on modern Windows 10 versions, but for ExternalData's use
case it is simpler to just not replace existing objects.
This commit is contained in:
Brad King 2021-03-04 09:06:34 -05:00
parent 15610d42fe
commit fdfbf89f0c

View File

@ -1101,7 +1101,14 @@ function(_ExternalData_download_object name hash algo var_obj var_success var_er
set(success 1)
if(found)
file(RENAME "${tmp}" "${obj}")
# Atomically create the object. If we lose a race with another process,
# do not replace it. Content-addressing ensures it has what we expect.
file(RENAME "${tmp}" "${obj}" NO_REPLACE RESULT result)
if (result STREQUAL "NO_REPLACE")
file(REMOVE "${tmp}")
elseif (result)
message(FATAL_ERROR "Failed to rename:\n \"${tmp}\"\nto:\n \"${obj}\"\nwith error:\n ${result}")
endif()
message(STATUS "Downloaded object: \"${obj}\"")
elseif(EXISTS "${staged}")
set(obj "${staged}")