# HG changeset patch
# User Alain Mazy <am@osimis.io>
# Date 1674234642 -3600
# Node ID 023569e7155bf17c3bfb4acf2b7438dffe61d883
# Parent  b2b38f9fb9d12c26ca6453b68572bb93ea4ad4e5
moved DicomModification thread safety into ResourceModificationJob (trying to avoid mutex in OrthancFramework as much as possible)

diff -r b2b38f9fb9d1 -r 023569e7155b OrthancFramework/Sources/DicomParsing/DicomModification.cpp
--- 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)
     {
diff -r b2b38f9fb9d1 -r 023569e7155b OrthancFramework/Sources/DicomParsing/DicomModification.h
--- 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_;
diff -r b2b38f9fb9d1 -r 023569e7155b OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp
--- 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_
           }
         }
       }
diff -r b2b38f9fb9d1 -r 023569e7155b OrthancServer/Sources/ServerJobs/ResourceModificationJob.h
--- 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
   };
 }