Mercurial > hg > orthanc

changeset 6574:dfb5748c8209

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
protect accesses to SharedArchive using readers-writer lock
author Sebastien Jodogne <s.jodogne@gmail.com>
date Tue, 27 Jan 2026 13:00:23 +0100
parents a96b627e2723
children 0000864c1f63
files OrthancFramework/Sources/Cache/SharedArchive.cpp OrthancFramework/Sources/Cache/SharedArchive.h OrthancServer/Sources/ServerJobs/ArchiveJob.cpp
diffstat 3 files changed, 31 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancFramework/Sources/Cache/SharedArchive.cpp	Tue Jan 27 12:06:49 2026 +0100
+++ b/OrthancFramework/Sources/Cache/SharedArchive.cpp	Tue Jan 27 13:00:23 2026 +0100
@@ -32,6 +32,9 @@
 {
   void SharedArchive::RemoveInternal(const std::string& id)
   {
+    // This function assumes that "mutex_" is locked by a WriterLock,
+    // and that "lruMutex_" is locked
+
     Archive::iterator it = archive_.find(id);
 
     if (it != archive_.end())
@@ -45,10 +48,9 @@
 
 
   SharedArchive::Accessor::Accessor(SharedArchive& that,
-                                    const std::string& id)
+                                    const std::string& id) :
+    lock_(that.mutex_)
   {
-    boost::recursive_mutex::scoped_lock lock(that.mutex_);
-
     Archive::iterator it = that.archive_.find(id);
 
     if (it == that.archive_.end())
@@ -57,7 +59,11 @@
     }
     else
     {
-      that.lru_.MakeMostRecent(id);
+      {
+        boost::mutex::scoped_lock lock(that.lruMutex_);
+        that.lru_.MakeMostRecent(id);
+      }
+
       item_ = it->second;
     }
   }
@@ -104,7 +110,8 @@
 
   std::string SharedArchive::Add(IDynamicObject* obj)
   {
-    boost::recursive_mutex::scoped_lock lock(mutex_);
+    WriterLock lock(mutex_);
+    boost::mutex::scoped_lock lruLock(lruMutex_);
 
     if (archive_.size() == maxSize_)
     {
@@ -124,7 +131,9 @@
 
   void SharedArchive::Remove(const std::string& id)
   {
-    boost::recursive_mutex::scoped_lock lock(mutex_);
+    WriterLock lock(mutex_);
+    boost::mutex::scoped_lock lruLock(lruMutex_);
+
     RemoveInternal(id);      
   }
 
@@ -134,7 +143,7 @@
     items.clear();
 
     {
-      boost::recursive_mutex::scoped_lock lock(mutex_);
+      ReaderLock lock(mutex_);
 
       for (Archive::const_iterator it = archive_.begin();
            it != archive_.end(); ++it)
--- a/OrthancFramework/Sources/Cache/SharedArchive.h	Tue Jan 27 12:06:49 2026 +0100
+++ b/OrthancFramework/Sources/Cache/SharedArchive.h	Tue Jan 27 13:00:23 2026 +0100
@@ -44,10 +44,15 @@
   {
   private:
     typedef std::map<std::string, IDynamicObject*>  Archive;
+    typedef boost::shared_lock<boost::shared_mutex> ReaderLock;
+    typedef boost::unique_lock<boost::shared_mutex> WriterLock;
 
     size_t                  maxSize_;
-    boost::recursive_mutex  mutex_;
+    boost::shared_mutex     mutex_;
     Archive                 archive_;
+
+    // The LRU index is not protected by "mutex_", but by "lruMutex_"
+    boost::mutex                        lruMutex_;
     LeastRecentlyUsedIndex<std::string> lru_;
 
     void RemoveInternal(const std::string& id);
@@ -56,8 +61,8 @@
     class ORTHANC_PUBLIC Accessor : public boost::noncopyable
     {
     private:
-      boost::recursive_mutex::scoped_lock lock_;
-      IDynamicObject*                     item_;
+      ReaderLock       lock_;
+      IDynamicObject*  item_;
 
     public:
       Accessor(SharedArchive& that,
@@ -68,7 +73,6 @@
       IDynamicObject& GetItem() const;
     };
 
-
     explicit SharedArchive(size_t maxSize);
 
     ~SharedArchive();
@@ -80,5 +84,3 @@
     void List(std::list<std::string>& items);
   };
 }
-
-
--- a/OrthancServer/Sources/ServerJobs/ArchiveJob.cpp	Tue Jan 27 12:06:49 2026 +0100
+++ b/OrthancServer/Sources/ServerJobs/ArchiveJob.cpp	Tue Jan 27 13:00:23 2026 +0100
@@ -1649,18 +1649,16 @@
     if (key == "archive" &&
         !mediaArchiveId_.empty())
     {
-      SharedArchive::Accessor accessor(context_.GetMediaArchive(), mediaArchiveId_);
+      bool found;
 
-      if (accessor.IsValid())
       {
-        context_.GetMediaArchive().Remove(mediaArchiveId_);
-        return true;
+        SharedArchive::Accessor accessor(context_.GetMediaArchive(), mediaArchiveId_);
+        found = accessor.IsValid();
       }
-      else
-      {
-        return false;
-      }
-    }    
+
+      context_.GetMediaArchive().Remove(mediaArchiveId_);
+      return found;
+    }
     else
     {
       return false;