From 917b83f149c625527f872fb2cf41289358fa5371 Mon Sep 17 00:00:00 2001
From: Pascal <admin@serveurperso.com>
Date: Fri, 11 Sep 2026 22:37:34 +0200
Subject: [PATCH] download: keep concurrent downloads of one blob apart

Every process writes the same path + .downloadInProgress, so a second
download of the same blob finds that file, takes it for its own partial
transfer and asks for the bytes after it, which produces a corrupt
result. The in-progress file now carries the pid of the process writing
it.

std::rename also replaces an existing destination on POSIX but fails on
Windows, so a download whose blob appeared in the meantime is dropped
after every retry and an etag rewrite silently keeps the old value.
std::filesystem::rename has the POSIX behaviour everywhere, and the
error now carries the reason reported by the system.
---
 common/download.cpp | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/common/download.cpp b/common/download.cpp
index 4b28a708c8..708837ad0a 100644
--- a/common/download.cpp
+++ b/common/download.cpp
@@ -38,6 +38,7 @@
 // isatty
 #if defined(_WIN32)
 #include <io.h>
+#include <process.h>
 #else
 #include <unistd.h>
 #endif
@@ -46,6 +47,17 @@
 // downloader
 //
 
+// the in-progress file of a download is private to the process writing it, so
+// two downloads of the same blob never share a partial file
+static std::string download_temporary_path(const std::string & path) {
+#ifdef _WIN32
+    const int pid = _getpid();
+#else
+    const int pid = getpid();
+#endif
+    return path + "." + std::to_string(pid) + ".downloadInProgress";
+}
+
 // validate repo name format: owner/repo
 static void write_file(const std::string & fname, const std::string & content) {
     const std::string fname_tmp = fname + ".tmp";
@@ -58,9 +70,13 @@ static void write_file(const std::string & fname, const std::string & content) {
         file << content;
         file.close();
 
-        // Makes write atomic
-        if (rename(fname_tmp.c_str(), fname.c_str()) != 0) {
-            LOG_ERR("%s: unable to rename file: %s to %s\n", __func__, fname_tmp.c_str(), fname.c_str());
+        // makes the write atomic, and replaces an existing destination on
+        // every platform, which std::rename does not do on Windows
+        std::error_code ec;
+        std::filesystem::rename(fname_tmp, fname, ec);
+        if (ec) {
+            LOG_ERR("%s: unable to rename file: %s to %s (%s)\n",
+                    __func__, fname_tmp.c_str(), fname.c_str(), ec.message().c_str());
             // If rename fails, try to delete the temporary file
             if (remove(fname_tmp.c_str()) != 0) {
                 LOG_ERR("%s: unable to delete temporary file: %s\n", __func__, fname_tmp.c_str());
@@ -366,7 +382,7 @@ static int common_download_file_single_online(const std::string & url,
     }
 
     bool success = false;
-    const std::string path_temporary = path + ".downloadInProgress";
+    const std::string path_temporary = download_temporary_path(path);
     int delay = retry_delay_seconds;
 
     if (opts.callback) {
@@ -401,8 +417,13 @@ static int common_download_file_single_online(const std::string & url,
                 path_temporary.c_str(), etag.c_str());
 
         if (common_pull_file(cli, parts.path, path_temporary, supports_ranges, p, opts.callback)) {
-            if (std::rename(path_temporary.c_str(), path.c_str()) != 0) {
-                LOG_ERR("%s: unable to rename file: %s to %s\n", __func__, path_temporary.c_str(), path.c_str());
+            // replaces an existing destination on every platform, which
+            // std::rename does not do on Windows
+            std::error_code ec;
+            std::filesystem::rename(path_temporary, path, ec);
+            if (ec) {
+                LOG_ERR("%s: unable to rename file: %s to %s (%s)\n",
+                        __func__, path_temporary.c_str(), path.c_str(), ec.message().c_str());
                 break;
             }
             if (!etag.empty() && !skip_etag) {