changeset 4459:16392fe89ce0

new mutex to protect registration of REST callbacks: restCallbackRegistrationMutex_
author Sebastien Jodogne <s.jodogne@gmail.com>
date Wed, 20 Jan 2021 13:30:54 +0100
parents e4dae17035b9
children 6831de40acd9
files OrthancServer/Plugins/Engine/OrthancPlugins.cpp TODO
diffstat 2 files changed, 35 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Wed Jan 20 12:43:44 2021 +0100
+++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Wed Jan 20 13:30:54 2021 +0100
@@ -867,7 +867,7 @@
     private:
       boost::regex              regex_;
       OrthancPluginRestCallback callback_;
-      bool                      lock_;
+      bool                      mutualExclusion_;
 
       OrthancPluginErrorCode InvokeInternal(PluginHttpOutput& output,
                                             const std::string& flatUri,
@@ -881,10 +881,10 @@
     public:
       RestCallback(const char* regex,
                    OrthancPluginRestCallback callback,
-                   bool lockRestCallbacks) :
+                   bool mutualExclusion) :
         regex_(regex),
         callback_(callback),
-        lock_(lockRestCallbacks)
+        mutualExclusion_(mutualExclusion)
       {
       }
 
@@ -893,14 +893,14 @@
         return regex_;
       }
 
-      OrthancPluginErrorCode Invoke(boost::recursive_mutex& restCallbackMutex,
+      OrthancPluginErrorCode Invoke(boost::recursive_mutex& invokationMutex,
                                     PluginHttpOutput& output,
                                     const std::string& flatUri,
                                     const OrthancPluginHttpRequest& request)
       {
-        if (lock_)
+        if (mutualExclusion_)
         {
-          boost::recursive_mutex::scoped_lock lock(restCallbackMutex);
+          boost::recursive_mutex::scoped_lock lock(invokationMutex);
           return InvokeInternal(output, flatUri, request);
         }
         else
@@ -1108,7 +1108,8 @@
     std::unique_ptr<StorageAreaFactory>  storageArea_;
     std::set<std::string> authorizationTokens_;
 
-    boost::recursive_mutex restCallbackMutex_;
+    boost::recursive_mutex restCallbackInvokationMutex_;
+    boost::shared_mutex restCallbackRegistrationMutex_;  // New in Orthanc 1.9.0
     boost::recursive_mutex storedCallbackMutex_;
     boost::recursive_mutex changeCallbackMutex_;
     boost::mutex findCallbackMutex_;
@@ -1912,6 +1913,7 @@
     PImpl::ChunkedRestCallback* callback = NULL;
 
     // Loop over the callbacks registered by the plugins
+    boost::shared_lock<boost::shared_mutex> lock(pimpl_->restCallbackRegistrationMutex_);
     for (PImpl::ChunkedRestCallbacks::const_iterator it = pimpl_->chunkedRestCallbacks_.begin(); 
          it != pimpl_->chunkedRestCallbacks_.end(); ++it)
     {
@@ -1986,6 +1988,7 @@
     PImpl::RestCallback* callback = NULL;
 
     // Loop over the callbacks registered by the plugins
+    boost::shared_lock<boost::shared_mutex> lock(pimpl_->restCallbackRegistrationMutex_);
     for (PImpl::RestCallbacks::const_iterator it = pimpl_->restCallbacks_.begin(); 
          it != pimpl_->restCallbacks_.end(); ++it)
     {
@@ -2013,7 +2016,7 @@
 
     assert(callback != NULL);
     OrthancPluginErrorCode error = callback->Invoke
-      (pimpl_->restCallbackMutex_, pluginOutput, matcher.GetFlatUri(), converter.GetRequest());
+      (pimpl_->restCallbackInvokationMutex_, pluginOutput, matcher.GetFlatUri(), converter.GetRequest());
 
     pluginOutput.Close(error, GetErrorDictionary());
     return true;
@@ -2194,17 +2197,20 @@
 
 
   void OrthancPlugins::RegisterRestCallback(const void* parameters,
-                                            bool lock)
+                                            bool mutualExclusion)
   {
     const _OrthancPluginRestCallback& p = 
       *reinterpret_cast<const _OrthancPluginRestCallback*>(parameters);
 
     CLOG(INFO, PLUGINS) << "Plugin has registered a REST callback "
-                        << (lock ? "with" : "without")
+                        << (mutualExclusion ? "with" : "without")
                         << " mutual exclusion on: " 
                         << p.pathRegularExpression;
 
-    pimpl_->restCallbacks_.push_back(new PImpl::RestCallback(p.pathRegularExpression, p.callback, lock));
+    {
+      boost::unique_lock<boost::shared_mutex> lock(pimpl_->restCallbackRegistrationMutex_);
+      pimpl_->restCallbacks_.push_back(new PImpl::RestCallback(p.pathRegularExpression, p.callback, mutualExclusion));
+    }
   }
 
 
@@ -2216,7 +2222,10 @@
     CLOG(INFO, PLUGINS) << "Plugin has registered a REST callback for chunked streams on: " 
                         << p.pathRegularExpression;
 
-    pimpl_->chunkedRestCallbacks_.push_back(new PImpl::ChunkedRestCallback(p));
+    {
+      boost::unique_lock<boost::shared_mutex> lock(pimpl_->restCallbackRegistrationMutex_);
+      pimpl_->chunkedRestCallbacks_.push_back(new PImpl::ChunkedRestCallback(p));
+    }
   }
 
 
@@ -5326,6 +5335,7 @@
     PImpl::ChunkedRestCallback* callback = NULL;
 
     // Loop over the callbacks registered by the plugins
+    boost::shared_lock<boost::shared_mutex> lock(pimpl_->restCallbackRegistrationMutex_);
     for (PImpl::ChunkedRestCallbacks::const_iterator it = pimpl_->chunkedRestCallbacks_.begin(); 
          it != pimpl_->chunkedRestCallbacks_.end(); ++it)
     {
--- a/TODO	Wed Jan 20 12:43:44 2021 +0100
+++ b/TODO	Wed Jan 20 13:30:54 2021 +0100
@@ -25,6 +25,8 @@
 Mid-term
 --------
 
+* Create DICOM from DICOMweb JSON ("application/dicom+json")
+  with "/tools/create-dicom"
 * Create multi-frame images with /tools/create-dicom (by adding a
   "MultiFrame" flag to avoid creating a series)
 * In the /studies/{id}/anonymize route, add an option to remove secondary captures.
@@ -142,6 +144,7 @@
 * Avoid direct calls to FromDcmtkBridge (make most of its 
   methods private), go through ParsedDicomFile wherever possible
 
+
 =================
 Platform-specific
 =================
@@ -180,6 +183,16 @@
 Misc
 ====
 
+-----
+Legal
+-----
+
+* Drop "OpenSSL Exception" from the Orthanc license once OpenSSL 3.0.0
+  is released, because that version will replace "OpenSSL license"
+  (incompatible with GPL) by "Apache License 2.0" (compatible with
+  GPL): https://people.gnome.org/~markmc/openssl-and-the-gpl.html
+  
+
 -----------
 Performance
 -----------