Mercurial > hg > orthanc

changeset 5175:48005e522bd6

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
start fixing thread safety issues with DicomMap
author Sebastien Jodogne <s.jodogne@gmail.com>
date Fri, 24 Mar 2023 19:00:33 +0100
parents d75af1cde602
children 6d8647122ef3
files OrthancFramework/Sources/DicomFormat/DicomMap.cpp OrthancFramework/Sources/DicomFormat/DicomMap.h OrthancFramework/UnitTestsSources/DicomMapTests.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp
diffstat 5 files changed, 70 insertions(+), 60 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancFramework/Sources/DicomFormat/DicomMap.cpp	Wed Mar 22 08:26:41 2023 +0100
+++ b/OrthancFramework/Sources/DicomFormat/DicomMap.cpp	Fri Mar 24 19:00:33 2023 +0100
@@ -139,8 +139,6 @@
   class DicomMap::MainDicomTagsConfiguration
   {
   private:
-    friend DicomMap;
-
     std::set<DicomTag>               patientsMainDicomTagsByLevel_;
     std::set<DicomTag>               studiesMainDicomTagsByLevel_;
     std::set<DicomTag>               seriesMainDicomTagsByLevel_;
@@ -156,27 +154,6 @@
       ResetDefaultMainDicomTags();
     }
 
-    void ResetDefaultMainDicomTags()
-    {
-      patientsMainDicomTagsByLevel_.clear();
-      studiesMainDicomTagsByLevel_.clear();
-      seriesMainDicomTagsByLevel_.clear();
-      instancesMainDicomTagsByLevel_.clear();
-
-      allMainDicomTags_.clear();
-
-      // by default, initialize with the previous static list (up to 1.10.0)
-      LoadDefaultMainDicomTags(ResourceType_Patient);
-      LoadDefaultMainDicomTags(ResourceType_Study);
-      LoadDefaultMainDicomTags(ResourceType_Series);
-      LoadDefaultMainDicomTags(ResourceType_Instance);
-
-      defaultSignatures_[ResourceType_Patient] = signatures_[ResourceType_Patient];
-      defaultSignatures_[ResourceType_Study] = signatures_[ResourceType_Study];
-      defaultSignatures_[ResourceType_Series] = signatures_[ResourceType_Series];
-      defaultSignatures_[ResourceType_Instance] = signatures_[ResourceType_Instance];
-    }
-
     std::string ComputeSignature(const std::set<DicomTag>& tags)
     {
       // std::set are sorted by default (which is important for us !)
@@ -229,29 +206,6 @@
       {
         AddMainDicomTag(tags[i], level);
       }
-
-    }
-
-
-    std::set<DicomTag>& GetMainDicomTagsByLevel(ResourceType level)
-    {
-      switch (level)
-      {
-        case ResourceType_Patient:
-          return patientsMainDicomTagsByLevel_;
-
-        case ResourceType_Study:
-          return studiesMainDicomTagsByLevel_;
-
-        case ResourceType_Series:
-          return seriesMainDicomTagsByLevel_;
-
-        case ResourceType_Instance:
-          return instancesMainDicomTagsByLevel_;
-
-        default:
-          throw OrthancException(ErrorCode_InternalError);
-      }
     }
 
   public:
@@ -262,6 +216,27 @@
       return parameters;
     }
 
+    void ResetDefaultMainDicomTags()
+    {
+      patientsMainDicomTagsByLevel_.clear();
+      studiesMainDicomTagsByLevel_.clear();
+      seriesMainDicomTagsByLevel_.clear();
+      instancesMainDicomTagsByLevel_.clear();
+
+      allMainDicomTags_.clear();
+
+      // by default, initialize with the previous static list (up to 1.10.0)
+      LoadDefaultMainDicomTags(ResourceType_Patient);
+      LoadDefaultMainDicomTags(ResourceType_Study);
+      LoadDefaultMainDicomTags(ResourceType_Series);
+      LoadDefaultMainDicomTags(ResourceType_Instance);
+
+      defaultSignatures_[ResourceType_Patient] = signatures_[ResourceType_Patient];
+      defaultSignatures_[ResourceType_Study] = signatures_[ResourceType_Study];
+      defaultSignatures_[ResourceType_Series] = signatures_[ResourceType_Series];
+      defaultSignatures_[ResourceType_Instance] = signatures_[ResourceType_Instance];
+    }
+
     void AddMainDicomTag(const DicomTag& tag, ResourceType level)
     {
       const std::set<DicomTag>& existingLevelTags = GetMainDicomTagsByLevel(level);
@@ -296,6 +271,26 @@
       return defaultSignatures_[level];
     }
 
+    std::set<DicomTag>& GetMainDicomTagsByLevel(ResourceType level)
+    {
+      switch (level)
+      {
+        case ResourceType_Patient:
+          return patientsMainDicomTagsByLevel_;
+
+        case ResourceType_Study:
+          return studiesMainDicomTagsByLevel_;
+
+        case ResourceType_Series:
+          return seriesMainDicomTagsByLevel_;
+
+        case ResourceType_Instance:
+          return instancesMainDicomTagsByLevel_;
+
+        default:
+          throw OrthancException(ErrorCode_InternalError);
+      }
+    }
   };
 
 
@@ -706,9 +701,10 @@
   }
 
 
-  const std::set<DicomTag>& DicomMap::GetMainDicomTags(ResourceType level)
+  void DicomMap::GetMainDicomTags(std::set<DicomTag>& target,
+                                  ResourceType level)
   {
-    return DicomMap::MainDicomTagsConfiguration::GetInstance().GetMainDicomTagsByLevel(level);
+    target = DicomMap::MainDicomTagsConfiguration::GetInstance().GetMainDicomTagsByLevel(level);
   }
 
   const std::set<DicomTag>& DicomMap::GetAllMainDicomTags()
--- a/OrthancFramework/Sources/DicomFormat/DicomMap.h	Wed Mar 22 08:26:41 2023 +0100
+++ b/OrthancFramework/Sources/DicomFormat/DicomMap.h	Fri Mar 24 19:00:33 2023 +0100
@@ -154,7 +154,8 @@
 
     static bool HasComputedTags(const std::set<DicomTag>& tags);
 
-    static const std::set<DicomTag>& GetMainDicomTags(ResourceType level);
+    static void GetMainDicomTags(std::set<DicomTag>& target,
+                                 ResourceType level);
 
     // returns a string uniquely identifying the list of main dicom tags for a level
     static const std::string& GetMainDicomTagsSignature(ResourceType level);
--- a/OrthancFramework/UnitTestsSources/DicomMapTests.cpp	Wed Mar 22 08:26:41 2023 +0100
+++ b/OrthancFramework/UnitTestsSources/DicomMapTests.cpp	Fri Mar 24 19:00:33 2023 +0100
@@ -90,26 +90,30 @@
     }
 
     {
-      const std::set<DicomTag>& s = DicomMap::GetMainDicomTags(ResourceType_Patient);
+      std::set<DicomTag> s;
+      DicomMap::GetMainDicomTags(s, ResourceType_Patient);
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_PATIENT_ID));
       ASSERT_TRUE(s.end() == s.find(DICOM_TAG_STUDY_INSTANCE_UID));
     }
 
     {
-      const std::set<DicomTag>& s = DicomMap::GetMainDicomTags(ResourceType_Study);
+      std::set<DicomTag> s;
+      DicomMap::GetMainDicomTags(s, ResourceType_Study);
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_STUDY_INSTANCE_UID));
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_ACCESSION_NUMBER));
       ASSERT_TRUE(s.end() == s.find(DICOM_TAG_PATIENT_ID));
     }
 
     {
-      const std::set<DicomTag>& s = DicomMap::GetMainDicomTags(ResourceType_Series);
+      std::set<DicomTag> s;
+      DicomMap::GetMainDicomTags(s, ResourceType_Series);
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_SERIES_INSTANCE_UID));
       ASSERT_TRUE(s.end() == s.find(DICOM_TAG_PATIENT_ID));
     }
 
     {
-      const std::set<DicomTag>& s = DicomMap::GetMainDicomTags(ResourceType_Instance);
+      std::set<DicomTag> s;
+      DicomMap::GetMainDicomTags(s, ResourceType_Instance);
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_SOP_INSTANCE_UID));
       ASSERT_TRUE(s.end() == s.find(DICOM_TAG_PATIENT_ID));
     }
@@ -120,12 +124,14 @@
     DicomMap::AddMainDicomTag(DICOM_TAG_BITS_ALLOCATED, ResourceType_Instance);
 
     {
-      const std::set<DicomTag>& s = DicomMap::GetMainDicomTags(ResourceType_Instance);
+      std::set<DicomTag> s;
+      DicomMap::GetMainDicomTags(s, ResourceType_Instance);
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_BITS_ALLOCATED));
       ASSERT_TRUE(s.end() != s.find(DICOM_TAG_SOP_INSTANCE_UID));
     }
     {
-      const std::set<DicomTag>& s = DicomMap::GetMainDicomTags(ResourceType_Series);
+      std::set<DicomTag> s;
+      DicomMap::GetMainDicomTags(s, ResourceType_Series);
       ASSERT_TRUE(s.end() == s.find(DICOM_TAG_BITS_ALLOCATED));
     }
 
@@ -242,8 +248,10 @@
   // REFERENCE: DICOM PS3.3 2015c - Information Object Definitions
   // http://dicom.nema.org/medical/dicom/current/output/html/part03.html
 
+  std::set<DicomTag> main;
+  DicomMap::GetMainDicomTags(main, level);
+
   std::set<DicomTag> moduleTags;
-  const std::set<DicomTag>& main = DicomMap::GetMainDicomTags(level);
   DicomTag::AddTagsForModule(moduleTags, module);
   
   // The main dicom tags are a subset of the module
@@ -918,7 +926,8 @@
   {
     ResourceType level = static_cast<ResourceType>(i);
 
-    const std::set<DicomTag>& tags = DicomMap::GetMainDicomTags(level);
+    std::set<DicomTag> tags;
+    DicomMap::GetMainDicomTags(tags, level);
 
     for (std::set<DicomTag>::const_iterator it = tags.begin(); it != tags.end(); ++it)
     {
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Wed Mar 22 08:26:41 2023 +0100
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Fri Mar 24 19:00:33 2023 +0100
@@ -362,7 +362,8 @@
       }
 
       {
-        const std::set<DicomTag>& tags = DicomMap::GetMainDicomTags(level);
+        std::set<DicomTag> tags;
+        DicomMap::GetMainDicomTags(tags, level);
 
         for (std::set<DicomTag>::const_iterator
                tag = tags.begin(); tag != tags.end(); ++tag)
--- a/OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp	Wed Mar 22 08:26:41 2023 +0100
+++ b/OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp	Fri Mar 24 19:00:33 2023 +0100
@@ -749,8 +749,11 @@
               DicomMap targetPatientTags;
               targetPatient.tags_.ExtractPatientInformation(targetPatientTags);
 
-              for (std::set<DicomTag>::const_iterator mainPatientTag = DicomMap::GetMainDicomTags(ResourceType_Patient).begin();
-                    mainPatientTag != DicomMap::GetMainDicomTags(ResourceType_Patient).end(); ++mainPatientTag)
+              std::set<DicomTag> mainPatientTags;
+              DicomMap::GetMainDicomTags(mainPatientTags, ResourceType_Patient);
+              
+              for (std::set<DicomTag>::const_iterator mainPatientTag = mainPatientTags.begin();
+                   mainPatientTag != mainPatientTags.end(); ++mainPatientTag)
               {
                 if (targetPatientTags.HasTag(*mainPatientTag) 
                     && (!modification_->IsReplaced(*mainPatientTag)