libstore: fix data race in getFileTransfer() singleton replacement

Multiple threads could simultaneously observe that the singleton
FileTransfer instance has quit and attempt to replace it without
synchronization. This caused undefined behavior as destroying the
old object while other threads might still be accessing its mutex
is not allowed.

Fixed by implementing in-place restart of the FileTransfer object
instead of replacing it. This avoids destroying mutexes that other
threads might be waiting on or holding.

Author: Mic92

src/libstore/filetransfer.cc

@@ -611,11 +611,10 @@ struct curlFileTransfer : public FileTransfer
 
     std::thread workerThread;
 
-    curlFileTransfer()
-        : mt19937(rd())
+    void resetCurl()
     {
-        static std::once_flag globalInit;
-        std::call_once(globalInit, curl_global_init, CURL_GLOBAL_ALL);
+        if (curlm)
+            curl_multi_cleanup(curlm);
 
         curlm = curl_multi_init();
 
@@ -625,13 +624,27 @@ struct curlFileTransfer : public FileTransfer
 #if LIBCURL_VERSION_NUM >= 0x071e00 // Max connections requires >= 7.30.0
         curl_multi_setopt(curlm, CURLMOPT_MAX_TOTAL_CONNECTIONS, fileTransferSettings.httpConnections.get());
 #endif
+    }
+
+    void startWorkerThread()
+    {
+        workerThread = std::thread([&]() { workerThreadEntry(); });
+    }
+
+    curlFileTransfer()
+        : mt19937(rd())
+    {
+        static std::once_flag globalInit;
+        std::call_once(globalInit, curl_global_init, CURL_GLOBAL_ALL);
+
+        resetCurl();
 
 #ifndef _WIN32 // TODO need graceful async exit support on Windows?
         wakeupPipe.create();
         fcntl(wakeupPipe.readSide.get(), F_SETFL, O_NONBLOCK);
 #endif
 
-        workerThread = std::thread([&]() { workerThreadEntry(); });
+        startWorkerThread();
     }
 
     ~curlFileTransfer()
@@ -644,6 +657,37 @@ struct curlFileTransfer : public FileTransfer
             curl_multi_cleanup(curlm);
     }
 
+    bool restarting = false;
+
+    void restart()
+    {
+        {
+            auto state(state_.lock());
+
+            // Check if we need to restart or if another thread is already doing it
+            if (!state->quit || restarting) {
+                return;
+            }
+
+            // Mark that we're restarting to prevent other threads from doing it
+            restarting = true;
+        } // state lock released here
+
+        // The worker thread has already exited (it set quit = true when it exited)
+        workerThread.join();
+
+        resetCurl();
+
+        // Reset the flags BEFORE starting the new thread
+        {
+            auto state(state_.lock());
+            state->quit = false;
+            restarting = false;
+        } // state lock released here
+
+        startWorkerThread();
+    }
+
     void stopWorkerThread()
     {
         /* Signal the worker thread to exit. */
@@ -845,8 +889,9 @@ ref<FileTransfer> getFileTransfer()
 {
     static ref<curlFileTransfer> fileTransfer = makeCurlFileTransfer();
 
-    if (fileTransfer->state_.lock()->quit)
-        fileTransfer = makeCurlFileTransfer();
+    if (fileTransfer->state_.lock()->quit) {
+        fileTransfer->restart();
+    }
 
     return fileTransfer;
 }