# HG changeset patch # User Sebastien Jodogne # Date 1679680833 -3600 # Node ID 48005e522bd683008d2b6730fc6a36e220b14b65 # Parent d75af1cde602ebc34903a2d0ca5e80733c393a9a start fixing thread safety issues with DicomMap diff -r d75af1cde602 -r 48005e522bd6 OrthancFramework/Sources/DicomFormat/DicomMap.cpp --- 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 patientsMainDicomTagsByLevel_; std::set studiesMainDicomTagsByLevel_; std::set 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& tags) { // std::set are sorted by default (which is important for us !) @@ -229,29 +206,6 @@ { AddMainDicomTag(tags[i], level); } - - } - - - std::set& 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& existingLevelTags = GetMainDicomTagsByLevel(level); @@ -296,6 +271,26 @@ return defaultSignatures_[level]; } + std::set& 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& DicomMap::GetMainDicomTags(ResourceType level) + void DicomMap::GetMainDicomTags(std::set& target, + ResourceType level) { - return DicomMap::MainDicomTagsConfiguration::GetInstance().GetMainDicomTagsByLevel(level); + target = DicomMap::MainDicomTagsConfiguration::GetInstance().GetMainDicomTagsByLevel(level); } const std::set& DicomMap::GetAllMainDicomTags() diff -r d75af1cde602 -r 48005e522bd6 OrthancFramework/Sources/DicomFormat/DicomMap.h --- 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& tags); - static const std::set& GetMainDicomTags(ResourceType level); + static void GetMainDicomTags(std::set& target, + ResourceType level); // returns a string uniquely identifying the list of main dicom tags for a level static const std::string& GetMainDicomTagsSignature(ResourceType level); diff -r d75af1cde602 -r 48005e522bd6 OrthancFramework/UnitTestsSources/DicomMapTests.cpp --- 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& s = DicomMap::GetMainDicomTags(ResourceType_Patient); + std::set 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& s = DicomMap::GetMainDicomTags(ResourceType_Study); + std::set 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& s = DicomMap::GetMainDicomTags(ResourceType_Series); + std::set 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& s = DicomMap::GetMainDicomTags(ResourceType_Instance); + std::set 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& s = DicomMap::GetMainDicomTags(ResourceType_Instance); + std::set 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& s = DicomMap::GetMainDicomTags(ResourceType_Series); + std::set 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 main; + DicomMap::GetMainDicomTags(main, level); + std::set moduleTags; - const std::set& 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(i); - const std::set& tags = DicomMap::GetMainDicomTags(level); + std::set tags; + DicomMap::GetMainDicomTags(tags, level); for (std::set::const_iterator it = tags.begin(); it != tags.end(); ++it) { diff -r d75af1cde602 -r 48005e522bd6 OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp --- 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& tags = DicomMap::GetMainDicomTags(level); + std::set tags; + DicomMap::GetMainDicomTags(tags, level); for (std::set::const_iterator tag = tags.begin(); tag != tags.end(); ++tag) diff -r d75af1cde602 -r 48005e522bd6 OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp --- 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::const_iterator mainPatientTag = DicomMap::GetMainDicomTags(ResourceType_Patient).begin(); - mainPatientTag != DicomMap::GetMainDicomTags(ResourceType_Patient).end(); ++mainPatientTag) + std::set mainPatientTags; + DicomMap::GetMainDicomTags(mainPatientTags, ResourceType_Patient); + + for (std::set::const_iterator mainPatientTag = mainPatientTags.begin(); + mainPatientTag != mainPatientTags.end(); ++mainPatientTag) { if (targetPatientTags.HasTag(*mainPatientTag) && (!modification_->IsReplaced(*mainPatientTag)