From fdfbf89f0c650a9886ffa91e4f3fda08080702b9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 Mar 2021 09:06:34 -0500 Subject: 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. --- Modules/ExternalData.cmake | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'Modules/ExternalData.cmake') 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}") -- cgit v1.2.1