changeset 5729:bdbaccc06e98

Fix extremely rare error when 2 threads are trying to create the same folder in the File Storage at the same time
author Alain Mazy <am@orthanc.team>
date Fri, 26 Jul 2024 16:29:10 +0200
parents 843973a0fdfa
children f0ab7a2678fe
files NEWS OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp
diffstat 2 files changed, 46 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Mon Jul 22 15:55:36 2024 +0200
+++ b/NEWS	Fri Jul 26 16:29:10 2024 +0200
@@ -19,6 +19,8 @@
   for a long period.
 * Fix C-Find queries not returning computed tags like ModalitiesInStudy, NumberOfStudyRelatedSeries, ...
   in very specific use-cases.
+* Fix extremely rare error when 2 threads are trying to create the same folder in the File Storage 
+  at the same time.
 
 
 
--- a/OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp	Mon Jul 22 15:55:36 2024 +0200
+++ b/OrthancFramework/Sources/FileStorage/FilesystemStorage.cpp	Fri Jul 26 16:29:10 2024 +0200
@@ -24,6 +24,7 @@
 
 #include "../PrecompiledHeaders.h"
 #include "FilesystemStorage.h"
+#include <boost/thread.hpp>
 
 // http://stackoverflow.com/questions/1576272/storing-large-number-of-files-in-file-system
 // http://stackoverflow.com/questions/446358/storing-a-large-number-of-images
@@ -135,25 +136,58 @@
     {
       // Extremely unlikely case: This Uuid has already been created
       // in the past.
-      throw OrthancException(ErrorCode_InternalError);
+      throw OrthancException(ErrorCode_InternalError, "This file UUID already exists");
     }
 
-    if (boost::filesystem::exists(path.parent_path()))
+    // In very unlikely case (but we've seen it !), 2 threads might enter this same piece
+    // of code and both try to create the same directory or a thread could be deleting a
+    // directory while another thread needs it -> introduce 3 retries at 1 ms interval
+    int retryCount = 0;
+    const int maxRetryCount = 3;
+    
+    while (retryCount < maxRetryCount)
     {
-      if (!boost::filesystem::is_directory(path.parent_path()))
+      retryCount++;
+      if (retryCount > 1)
       {
-        throw OrthancException(ErrorCode_DirectoryOverFile);
+        boost::this_thread::sleep(boost::posix_time::milliseconds(2 * retryCount + (rand() % 10)));
+        LOG(INFO) << "Retrying to create attachment \"" << uuid << "\" of \"" << GetDescriptionInternal(type) 
+                  << "\" type";
+      }
+
+      if (boost::filesystem::exists(path.parent_path()))
+      {
+        if (!boost::filesystem::is_directory(path.parent_path()))
+        {
+          throw OrthancException(ErrorCode_DirectoryOverFile);  // no need to retry this error
+        }
       }
-    }
-    else
-    {
-      if (!boost::filesystem::create_directories(path.parent_path()))
+      else
       {
-        throw OrthancException(ErrorCode_FileStorageCannotWrite);
+        if (!boost::filesystem::create_directories(path.parent_path()))
+        {
+          if (retryCount >= maxRetryCount)
+          {
+            throw OrthancException(ErrorCode_FileStorageCannotWrite);
+          }
+          
+          continue;  // retry
+        }
+      }
+
+      try 
+      {
+        SystemToolbox::WriteFile(content, size, path.string(), fsyncOnWrite_);
+      }
+      catch (OrthancException& ex)
+      {
+        if (retryCount >= maxRetryCount)
+        {
+          throw ex;
+        }
       }
     }
 
-    SystemToolbox::WriteFile(content, size, path.string(), fsyncOnWrite_);
     LOG(INFO) << "Created attachment \"" << uuid << "\" (" << timer.GetHumanTransferSpeed(true, size) << ")";
   }