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.

1 files changed, 8 insertions, 1 deletions

diff --git a/Modules/ExternalData.cmake b/Modules/ExternalData.cmake
index d6fbae9aaf..b439636f5c 100644
--- a/Modules/ExternalData.cmake
+++ b/Modules/ExternalData.cmake
@@ -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}")