Mercurial > hg > orthanc

changeset 6184:d0d08c3833d1

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
sharing classes DictionaryReaderLock and DictionaryWriterLock
author Sebastien Jodogne <s.jodogne@gmail.com>
date Thu, 12 Jun 2025 13:12:16 +0200
parents 39efb66b8322
children fc0be0031fe8 24fc490e40b7
files OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h OrthancServer/Plugins/Engine/OrthancPlugins.cpp
diffstat 3 files changed, 130 insertions(+), 120 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp	Thu Jun 12 12:18:20 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp	Thu Jun 12 13:12:16 2025 +0200
@@ -58,7 +58,6 @@
 
 #include <dcmtk/dcmdata/dcdeftag.h>
 #include <dcmtk/dcmdata/dcdicent.h>
-#include <dcmtk/dcmdata/dcdict.h>
 #include <dcmtk/dcmdata/dcfilefo.h>
 #include <dcmtk/dcmdata/dcistrmb.h>
 #include <dcmtk/dcmdata/dcostrmb.h>
@@ -127,6 +126,38 @@
 
 namespace Orthanc
 {
+  FromDcmtkBridge::DictionaryWriterLock::DictionaryWriterLock() :
+    dictionary_(dcmDataDict.wrlock())
+  {
+  }
+
+
+  FromDcmtkBridge::DictionaryWriterLock::~DictionaryWriterLock()
+  {
+#if DCMTK_VERSION_NUMBER >= 364
+    dcmDataDict.wrunlock();
+#else
+    dcmDataDict.unlock();
+#endif
+  }
+
+
+  FromDcmtkBridge::DictionaryReaderLock::DictionaryReaderLock() :
+    dictionary_(dcmDataDict.rdlock())
+  {
+  }
+
+
+  FromDcmtkBridge::DictionaryReaderLock::~DictionaryReaderLock()
+  {
+#if DCMTK_VERSION_NUMBER >= 364
+    dcmDataDict.rdunlock();
+#else
+    dcmDataDict.unlock();
+#endif
+  }
+
+
   static bool IsBinaryTag(const DcmTag& key)
   {
     return (key.isUnknownVR() ||
@@ -235,37 +266,6 @@
 
   namespace
   {
-    class DictionaryLocker : public boost::noncopyable
-    {
-    private:
-      DcmDataDictionary& dictionary_;
-
-    public:
-      DictionaryLocker() : dictionary_(dcmDataDict.wrlock())
-      {
-      }
-
-      ~DictionaryLocker()
-      {
-#if DCMTK_VERSION_NUMBER >= 364
-        dcmDataDict.wrunlock();
-#else
-        dcmDataDict.unlock();
-#endif
-      }
-
-      DcmDataDictionary& operator*()
-      {
-        return dictionary_;
-      }
-
-      DcmDataDictionary* operator->()
-      {
-        return &dictionary_;
-      }
-    };
-
-    
     ORTHANC_FORCE_INLINE
     static std::string FloatToString(float v)
     {
@@ -364,9 +364,9 @@
     
 #if DCMTK_USE_EMBEDDED_DICTIONARIES == 1
     {
-      DictionaryLocker locker;
-
-      locker->clear();
+      DictionaryWriterLock lock;
+
+      lock.GetDictionary().clear();
 
       CLOG(INFO, DICOM) << "Loading the embedded dictionaries";
       /**
@@ -374,14 +374,14 @@
        * command "strace storescu 2>&1 |grep dic" shows that DICONDE
        * dictionary is not loaded by storescu.
        **/
-      //LoadEmbeddedDictionary(*locker, FrameworkResources::DICTIONARY_DICONDE);
-
-      LoadEmbeddedDictionary(*locker, FrameworkResources::DICTIONARY_DICOM);
+      //LoadEmbeddedDictionary(*lock, FrameworkResources::DICTIONARY_DICONDE);
+
+      LoadEmbeddedDictionary(*lock, FrameworkResources::DICTIONARY_DICOM);
 
       if (loadPrivateDictionary)
       {
         CLOG(INFO, DICOM) << "Loading the embedded dictionary of private tags";
-        LoadEmbeddedDictionary(*locker, FrameworkResources::DICTIONARY_PRIVATE);
+        LoadEmbeddedDictionary(*lock, FrameworkResources::DICTIONARY_PRIVATE);
       }
       else
       {
@@ -441,16 +441,16 @@
 
   void FromDcmtkBridge::LoadExternalDictionaries(const std::vector<std::string>& dictionaries)
   {
-    DictionaryLocker locker;
+    DictionaryWriterLock lock;
 
     CLOG(INFO, DICOM) << "Clearing the DICOM dictionary";
-    locker->clear();
+    lock.GetDictionary().clear();
 
     for (size_t i = 0; i < dictionaries.size(); i++)
     {
       LOG(WARNING) << "Loading external DICOM dictionary: \"" << dictionaries[i] << "\"";
         
-      if (!locker->loadDictionary(dictionaries[i].c_str()))
+      if (!lock.GetDictionary().loadDictionary(dictionaries[i].c_str()))
       {
         throw OrthancException(ErrorCode_InexistentFile);
       }
@@ -543,10 +543,10 @@
     entry->setElementRangeRestriction(DcmDictRange_Unspecified);
 
     {
-      DictionaryLocker locker;
-
-      if (locker->findEntry(DcmTagKey(tag.GetGroup(), tag.GetElement()),
-                            privateCreator.empty() ? NULL : privateCreator.c_str()))
+      DictionaryWriterLock lock;
+
+      if (lock.GetDictionary().findEntry(DcmTagKey(tag.GetGroup(), tag.GetElement()),
+                                         privateCreator.empty() ? NULL : privateCreator.c_str()))
       {
         throw OrthancException(ErrorCode_AlreadyExistingTag,
                                "Cannot register twice the tag (" + tag.Format() +
@@ -554,7 +554,7 @@
       }
       else
       {
-        locker->addEntry(entry.release());
+        lock.GetDictionary().addEntry(entry.release());
       }
     }
   }
@@ -731,10 +731,11 @@
        * syntax (cf. DICOM CP 246).
        * ftp://medical.nema.org/medical/dicom/final/cp246_ft.pdf
        **/
-      DictionaryLocker locker;
-      
-      const DcmDictEntry* entry = locker->findEntry(element.getTag().getXTag(), 
-                                                    element.getTag().getPrivateCreator());
+      DictionaryReaderLock lock;
+
+      // The "entry" value is only valid while "lock" is active
+      const DcmDictEntry* entry = lock.GetDictionary().findEntry(element.getTag().getXTag(),
+                                                                 element.getTag().getPrivateCreator());
       if (entry != NULL && 
           entry->getVR().isaString())
       {
@@ -1179,8 +1180,8 @@
 
       if (!(flags & DicomToJsonFlags_IncludeUnknownTags))
       {
-        DictionaryLocker locker;
-        if (locker->findEntry(element->getTag(), element->getTag().getPrivateCreator()) == NULL)
+        DictionaryReaderLock lock;
+        if (lock.GetDictionary().findEntry(element->getTag(), element->getTag().getPrivateCreator()) == NULL)
         {
           continue;
         }
@@ -2759,10 +2760,11 @@
     if (evr == EVR_UN)
     {
       // New in Orthanc 1.9.5
-      DictionaryLocker locker;
-      
-      const DcmDictEntry* entry = locker->findEntry(element.getTag().getXTag(),
-                                                    element.getTag().getPrivateCreator());
+      FromDcmtkBridge::DictionaryReaderLock lock;
+
+      // The "entry" value is only valid while "lock" is active
+      const DcmDictEntry* entry = lock.GetDictionary().findEntry(element.getTag().getXTag(),
+                                                                 element.getTag().getPrivateCreator());
 
       if (entry != NULL)
       {
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h	Thu Jun 12 12:18:20 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.h	Thu Jun 12 13:12:16 2025 +0200
@@ -30,9 +30,10 @@
 #include "../DicomFormat/DicomPath.h"
 
 #include <dcmtk/dcmdata/dcdatset.h>
+#include <dcmtk/dcmdata/dcdict.h>
+#include <dcmtk/dcmdata/dcfilefo.h>
 #include <dcmtk/dcmdata/dcmetinf.h>
 #include <dcmtk/dcmdata/dcpixseq.h>
-#include <dcmtk/dcmdata/dcfilefo.h>
 #include <json/value.h>
 
 #if ORTHANC_ENABLE_DCMTK != 1
@@ -86,6 +87,40 @@
     };
     
 
+    class ORTHANC_PUBLIC DictionaryWriterLock : public boost::noncopyable
+    {
+    private:
+      DcmDataDictionary& dictionary_;
+
+    public:
+      DictionaryWriterLock();
+
+      ~DictionaryWriterLock();
+
+      DcmDataDictionary& GetDictionary()
+      {
+        return dictionary_;
+      }
+    };
+
+
+    class ORTHANC_PUBLIC DictionaryReaderLock : public boost::noncopyable
+    {
+    private:
+      const DcmDataDictionary& dictionary_;
+
+    public:
+      DictionaryReaderLock();
+
+      ~DictionaryReaderLock();
+
+      const DcmDataDictionary& GetDictionary() const
+      {
+        return dictionary_;
+      }
+    };
+
+
   private:
     FromDcmtkBridge();  // Pure static class
 
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Thu Jun 12 12:18:20 2025 +0200
+++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Thu Jun 12 13:12:16 2025 +0200
@@ -71,7 +71,6 @@
 
 #include <boost/math/special_functions/round.hpp>
 #include <boost/regex.hpp>
-#include <dcmtk/dcmdata/dcdict.h>
 #include <dcmtk/dcmdata/dcdicent.h>
 #include <dcmtk/dcmnet/dimse.h>
 
@@ -4535,35 +4534,6 @@
   }
 
 
-  namespace
-  {
-    class DictionaryReadLocker
-    {
-    private:
-      const DcmDataDictionary& dictionary_;
-
-    public:
-      DictionaryReadLocker() : dictionary_(dcmDataDict.rdlock())
-      {
-      }
-
-      ~DictionaryReadLocker()
-      {
-#if DCMTK_VERSION_NUMBER >= 364
-        dcmDataDict.rdunlock();
-#else
-        dcmDataDict.unlock();
-#endif
-      }
-
-      const DcmDataDictionary* operator->()
-      {
-        return &dictionary_;
-      }
-    };
-  }
-
-
   void OrthancPlugins::ApplyLookupDictionary(const void* parameters)
   {
     const _OrthancPluginLookupDictionary& p =
@@ -4572,38 +4542,41 @@
     DicomTag tag(FromDcmtkBridge::ParseTag(p.name));
     DcmTagKey tag2(tag.GetGroup(), tag.GetElement());
 
-    DictionaryReadLocker locker;
-    const DcmDictEntry* entry = NULL;
-
-    if (tag.IsPrivate())
-    {
-      // Fix issue 168 (Plugins can't read private tags from the
-      // configuration file)
-      // https://orthanc.uclouvain.be/bugs/show_bug.cgi?id=168
-      std::string privateCreator;
-      {
-        OrthancConfiguration::ReaderLock lock;
-        privateCreator = lock.GetConfiguration().GetDefaultPrivateCreator();
-      }
-
-      entry = locker->findEntry(tag2, privateCreator.c_str());
-    }
-    else
-    {
-      entry = locker->findEntry(tag2, NULL);
-    }
-
-    if (entry == NULL)
-    {
-      throw OrthancException(ErrorCode_UnknownDicomTag, p.name);
-    }
-    else
-    {
-      p.target->group = entry->getKey().getGroup();
-      p.target->element = entry->getKey().getElement();
-      p.target->vr = Plugins::Convert(FromDcmtkBridge::Convert(entry->getEVR()));
-      p.target->minMultiplicity = static_cast<uint32_t>(entry->getVMMin());
-      p.target->maxMultiplicity = (entry->getVMMax() == DcmVariableVM ? 0 : static_cast<uint32_t>(entry->getVMMax()));
+    {
+      FromDcmtkBridge::DictionaryReaderLock lock;
+
+      const DcmDictEntry* entry = NULL;  // This value is only valid while "lock" is active
+
+      if (tag.IsPrivate())
+      {
+        // Fix issue 168 (Plugins can't read private tags from the
+        // configuration file)
+        // https://orthanc.uclouvain.be/bugs/show_bug.cgi?id=168
+        std::string privateCreator;
+        {
+          OrthancConfiguration::ReaderLock configurationLock;
+          privateCreator = configurationLock.GetConfiguration().GetDefaultPrivateCreator();
+        }
+
+        entry = lock.GetDictionary().findEntry(tag2, privateCreator.c_str());
+      }
+      else
+      {
+        entry = lock.GetDictionary().findEntry(tag2, NULL);
+      }
+
+      if (entry == NULL)
+      {
+        throw OrthancException(ErrorCode_UnknownDicomTag, p.name);
+      }
+      else
+      {
+        p.target->group = entry->getKey().getGroup();
+        p.target->element = entry->getKey().getElement();
+        p.target->vr = Plugins::Convert(FromDcmtkBridge::Convert(entry->getEVR()));
+        p.target->minMultiplicity = static_cast<uint32_t>(entry->getVMMin());
+        p.target->maxMultiplicity = (entry->getVMMax() == DcmVariableVM ? 0 : static_cast<uint32_t>(entry->getVMMax()));
+      }
     }
   }