Mercurial > hg > orthanc

changeset 5141:023569e7155b

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
moved DicomModification thread safety into ResourceModificationJob (trying to avoid mutex in OrthancFramework as much as possible)
author Alain Mazy <am@osimis.io>
date Fri, 20 Jan 2023 18:10:42 +0100
parents b2b38f9fb9d1
children 45ce9dfa42e0
files OrthancFramework/Sources/DicomParsing/DicomModification.cpp OrthancFramework/Sources/DicomParsing/DicomModification.h OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp OrthancServer/Sources/ServerJobs/ResourceModificationJob.h
diffstat 4 files changed, 18 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.cpp	Fri Jan 20 17:24:35 2023 +0100
+++ b/OrthancFramework/Sources/DicomParsing/DicomModification.cpp	Fri Jan 20 18:10:42 2023 +0100
@@ -439,8 +439,6 @@
                                                         const std::string& mapped,
                                                         ResourceType level)
   {
-    boost::recursive_mutex::scoped_lock lock(uidMapMutex_);
-
     UidMap::const_iterator previous = uidMap_.find(std::make_pair(level, original));
 
     if (previous == uidMap_.end())
@@ -452,8 +450,6 @@
   std::string DicomModification::MapDicomIdentifier(const std::string& original,
                                                     ResourceType level)
   {
-    boost::recursive_mutex::scoped_lock lock(uidMapMutex_);
-
     const std::string stripped = Toolbox::StripSpaces(original);
     
     std::string mapped;
@@ -694,8 +690,6 @@
 
   void DicomModification::SetLevel(ResourceType level)
   {
-    boost::recursive_mutex::scoped_lock lock(uidMapMutex_);
-
     uidMap_.clear();
     level_ = level;
 
@@ -851,8 +845,6 @@
 
   void DicomModification::SetupAnonymization(DicomVersion version)
   {
-    boost::recursive_mutex::scoped_lock lock(uidMapMutex_);
-
     isAnonymization_ = true;
     
     keep_.clear();
@@ -1429,8 +1421,6 @@
   
   void DicomModification::Serialize(Json::Value& value) const
   {
-    boost::recursive_mutex::scoped_lock lock(uidMapMutex_);
-
     if (identifierGenerator_ != NULL)
     {
       throw OrthancException(ErrorCode_InternalError,
@@ -1556,8 +1546,6 @@
                                             const Json::Value& serialized,
                                             const char* field)
   {
-    boost::recursive_mutex::scoped_lock lock(uidMapMutex_);
-
     if (!serialized.isMember(field) ||
         serialized[field].type() != Json::objectValue)
     {
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.h	Fri Jan 20 17:24:35 2023 +0100
+++ b/OrthancFramework/Sources/DicomParsing/DicomModification.h	Fri Jan 20 18:10:42 2023 +0100
@@ -24,7 +24,6 @@
 #pragma once
 
 #include "ParsedDicomFile.h"
-#include <boost/thread/recursive_mutex.hpp>
 
 #include <list>
 
@@ -127,7 +126,6 @@
 
     typedef std::map< std::pair<ResourceType, std::string>, std::string>  UidMap;
     
-    mutable boost::recursive_mutex      uidMapMutex_;
     SetOfTags removals_;
     SetOfTags clearings_;
     SetOfTags keep_;
--- a/OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp	Fri Jan 20 17:24:35 2023 +0100
+++ b/OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp	Fri Jan 20 18:10:42 2023 +0100
@@ -230,7 +230,11 @@
      * Compute the resulting DICOM instance.
      **/
 
-    modification_->Apply(*modified);
+    {
+      boost::recursive_mutex::scoped_lock lock(mutex_);  // DicomModification object is not thread safe, we must protect it from here
+
+      modification_->Apply(*modified);
+    }
 
     const std::string modifiedUid = IDicomTranscoder::GetSopInstanceUid(modified->GetDcmtkObject());
     
@@ -406,6 +410,7 @@
   }
 
 
+#if ORTHANC_BUILD_UNIT_TESTS == 1
   const DicomModification& ResourceModificationJob::GetModification() const
   {
     if (modification_.get() == NULL)
@@ -417,7 +422,7 @@
       return *modification_;
     }
   }
-
+#endif
 
   DicomTransferSyntax ResourceModificationJob::GetTransferSyntax() const
   {
@@ -627,7 +632,13 @@
       origin_.Serialize(value[ORIGIN]);
       
       Json::Value tmp;
-      modification_->Serialize(tmp);
+
+      {
+        boost::recursive_mutex::scoped_lock lock(mutex_);  // DicomModification object is not thread safe, we must protect it from here
+  
+        modification_->Serialize(tmp);
+      }
+
       value[MODIFICATION] = tmp;
 
       // New in Orthanc 1.9.4
@@ -757,7 +768,6 @@
                 }
               }
             }
-            // TODO: need reconstruct_
           }
         }
       }
--- a/OrthancServer/Sources/ServerJobs/ResourceModificationJob.h	Fri Jan 20 17:24:35 2023 +0100
+++ b/OrthancServer/Sources/ServerJobs/ResourceModificationJob.h	Fri Jan 20 18:10:42 2023 +0100
@@ -87,8 +87,6 @@
 
     void SetOrigin(const RestApiCall& call);
 
-    const DicomModification& GetModification() const;
-
     bool IsAnonymization() const
     {
       return isAnonymization_;
@@ -129,5 +127,9 @@
     virtual void Reset() ORTHANC_OVERRIDE;
 
     void PerformSanityChecks();
+
+#if ORTHANC_BUILD_UNIT_TESTS == 1
+    const DicomModification& GetModification() const;
+#endif
   };
 }