changeset 778:aebf0071020e

refactoring of the mutex for the dicom cache
author Sebastien Jodogne <s.jodogne@gmail.com>
date Fri, 02 May 2014 11:02:23 +0200
parents be87dd517416
children e7eb70772fbe
files OrthancServer/OrthancRestApi/OrthancRestAnonymizeModify.cpp OrthancServer/ServerContext.cpp OrthancServer/ServerContext.h
diffstat 3 files changed, 56 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/OrthancRestApi/OrthancRestAnonymizeModify.cpp	Wed Apr 30 17:53:01 2014 +0200
+++ b/OrthancServer/OrthancRestApi/OrthancRestAnonymizeModify.cpp	Fri May 02 11:02:23 2014 +0200
@@ -36,22 +36,15 @@
 
 namespace Orthanc
 {
-  // TODO IMPROVE MULTITHREADING
-  // Every call to "ParsedDicomFile" must lock this mutex!!!
-  static boost::mutex cacheMutex_;
-
-
   // Raw access to the DICOM tags of an instance ------------------------------
 
   static void GetRawContent(RestApi::GetCall& call)
   {
-    boost::mutex::scoped_lock lock(cacheMutex_);
-
-    ServerContext& context = OrthancRestApi::GetContext(call);
+    std::string id = call.GetUriComponent("id", "");
 
-    std::string id = call.GetUriComponent("id", "");
-    ParsedDicomFile& dicom = context.GetDicomFile(id);
-    dicom.SendPathValue(call.GetOutput(), call.GetTrailingUri());
+    ServerContext::DicomCacheLocker locker(OrthancRestApi::GetContext(call), id);
+
+    locker.GetDicom().SendPathValue(call.GetOutput(), call.GetTrailingUri());
   }
 
 
@@ -338,13 +331,11 @@
                                         bool removePrivateTags,
                                         RestApi::PostCall& call)
   {
-    boost::mutex::scoped_lock lock(cacheMutex_);
-    ServerContext& context = OrthancRestApi::GetContext(call);
+    std::string id = call.GetUriComponent("id", "");
 
-    std::string id = call.GetUriComponent("id", "");
-    ParsedDicomFile& dicom = context.GetDicomFile(id);
-    
-    std::auto_ptr<ParsedDicomFile> modified(dicom.Clone());
+    ServerContext::DicomCacheLocker locker(OrthancRestApi::GetContext(call), id);
+
+    std::auto_ptr<ParsedDicomFile> modified(locker.GetDicom().Clone());
     ReplaceInstanceInternal(*modified, removals, replacements, DicomReplaceMode_InsertIfAbsent, removePrivateTags);
     modified->Answer(call.GetOutput());
   }
@@ -416,7 +407,6 @@
     bool isFirst = true;
     Json::Value result(Json::objectValue);
 
-    boost::mutex::scoped_lock lock(cacheMutex_);
     ServerContext& context = OrthancRestApi::GetContext(call);
 
     Instances instances;
@@ -437,7 +427,20 @@
          it != instances.end(); ++it)
     {
       LOG(INFO) << "Modifying instance " << *it;
-      ParsedDicomFile& original = context.GetDicomFile(*it);
+
+      std::auto_ptr<ServerContext::DicomCacheLocker> locker;
+
+      try
+      {
+        locker.reset(new ServerContext::DicomCacheLocker(OrthancRestApi::GetContext(call), *it));
+      }
+      catch (OrthancException&)
+      {
+        // This child instance has been removed in between
+        continue;
+      }
+
+      ParsedDicomFile& original = locker->GetDicom();
 
       DicomInstanceHasher originalHasher = original.GetHasher();
 
--- a/OrthancServer/ServerContext.cpp	Wed Apr 30 17:53:01 2014 +0200
+++ b/OrthancServer/ServerContext.cpp	Fri May 02 11:02:23 2014 +0200
@@ -222,14 +222,26 @@
   }
 
 
-  ParsedDicomFile& ServerContext::GetDicomFile(const std::string& instancePublicId)
+  ServerContext::DicomCacheLocker::DicomCacheLocker(ServerContext& that,
+                                                    const std::string& instancePublicId) : 
+    that_(that)
   {
 #if ENABLE_DICOM_CACHE == 0
     static std::auto_ptr<IDynamicObject> p;
     p.reset(provider_.Provide(instancePublicId));
-    return dynamic_cast<ParsedDicomFile&>(*p);
+    dicom_ = dynamic_cast<ParsedDicomFile*>(p.get());
 #else
-    return dynamic_cast<ParsedDicomFile&>(dicomCache_.Access(instancePublicId));
+    that_.dicomCacheMutex_.lock();
+    dicom_ = &dynamic_cast<ParsedDicomFile&>(that_.dicomCache_.Access(instancePublicId));
+#endif
+  }
+
+
+  ServerContext::DicomCacheLocker::~DicomCacheLocker()
+  {
+#if ENABLE_DICOM_CACHE == 0
+#else
+    that_.dicomCacheMutex_.unlock();
 #endif
   }
 
--- a/OrthancServer/ServerContext.h	Wed Apr 30 17:53:01 2014 +0200
+++ b/OrthancServer/ServerContext.h	Fri May 02 11:02:23 2014 +0200
@@ -70,12 +70,31 @@
     bool compressionEnabled_;
     
     DicomCacheProvider provider_;
+    boost::mutex dicomCacheMutex_;
     MemoryCache dicomCache_;
     ReusableDicomUserConnection scu_;
 
     LuaContext lua_;
 
   public:
+    class DicomCacheLocker
+    {
+    private:
+      ServerContext& that_;
+      ParsedDicomFile *dicom_;
+
+    public:
+      DicomCacheLocker(ServerContext& that,
+                       const std::string& instancePublicId);
+
+      ~DicomCacheLocker();
+
+      ParsedDicomFile& GetDicom()
+      {
+        return *dicom_;
+      }
+    };
+
     ServerContext(const boost::filesystem::path& storagePath,
                   const boost::filesystem::path& indexPath);
 
@@ -138,9 +157,6 @@
                   FileContentType content,
                   bool uncompressIfNeeded = true);
 
-    // TODO IMPLEMENT MULTITHREADING FOR THIS METHOD
-    ParsedDicomFile& GetDicomFile(const std::string& instancePublicId);
-
     LuaContext& GetLuaContext()
     {
       return lua_;