changeset 726:d757ef9df8a7

fixed a possible deadlock when WadoRsLoaderThreadsCount > 1
author Alain Mazy <am@orthanc.team>
date Fri, 24 Oct 2025 16:33:29 +0200
parents 31d0f5942b29
children 2bd27a266937 7c55a3a9af1c
files NEWS Plugin/WadoRs.cpp
diffstat 2 files changed, 45 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Wed Sep 24 09:00:36 2025 +0200
+++ b/NEWS	Fri Oct 24 16:33:29 2025 +0200
@@ -1,3 +1,10 @@
+Pending changes in the mainline
+===============================
+
+* Fixed a possible deadlock when using "WadoRsLoaderThreadsCount" > 1 when the HTTP
+  client disconnects while downloading the response.
+
+
 Version 1.21 (2025-08-14)
 =========================
 
--- a/Plugin/WadoRs.cpp	Wed Sep 24 09:00:36 2025 +0200
+++ b/Plugin/WadoRs.cpp	Fri Oct 24 16:33:29 2025 +0200
@@ -453,12 +453,15 @@
 
   Orthanc::Semaphore                  bufferSemaphore_;
 
+  bool                                loadersShouldStop_;
+
 public:
   ThreadedInstanceLoader(size_t threadCount, bool transcode, Orthanc::DicomTransferSyntax transferSyntax)
   : InstanceLoader(transcode, transferSyntax),
     instancesToPreload_(0),
     loadedInstances_(0),
-    bufferSemaphore_(3*threadCount) // to limit the number of loaded instances in memory
+    bufferSemaphore_(3*threadCount), // to limit the number of loaded instances in memory
+    loadersShouldStop_(false)
   {
     for (size_t i = 0; i < threadCount; i++)
     {
@@ -473,21 +476,38 @@
 
   void Clear()
   {
-    for (size_t i = 0; i < threads_.size(); i++)
-    {
-      instancesToPreload_.Enqueue(NULL);
-    }
-
-    for (size_t i = 0; i < threads_.size(); i++)
+    if (threads_.size() > 0)
     {
-      if (threads_[i]->joinable())
+      loadersShouldStop_ = true; // not need to protect this by a mutex.  This is the only "writer" and all loaders are "readers"
+
+      LOG(INFO) << "Waiting for loader threads to complete";
+
+      // unlock the loaders if they are waiting on this message queue (this happens when the job completes sucessfully)
+      for (size_t i = 0; i < threads_.size(); i++)
       {
-        threads_[i]->join();
+        instancesToPreload_.Enqueue(NULL);
       }
-      delete threads_[i];
+
+      // If the consumer stops e.g. because the HttpClient disconnected, we must make sure the loader threads are not blocked waiting for room in the bufferSemaphore_.
+      // If the loader threads have completed their jobs, this is harmless to release the bufferSemaphore_ since they won't be used anymore.
+      for (size_t i = 0; i < threads_.size(); i++)
+      {
+        bufferSemaphore_.Release();
+      }
+
+      for (size_t i = 0; i < threads_.size(); i++)
+      {
+        if (threads_[i]->joinable())
+        {
+          threads_[i]->join();
+        }
+        delete threads_[i];
+      }
+
+      threads_.clear();
+
+      LOG(INFO) << "Waiting for loader threads to complete - done";
     }
-
-    threads_.clear();
   }
 
   static void PreloaderWorkerThread(ThreadedInstanceLoader* that)
@@ -495,11 +515,14 @@
     static uint16_t threadCounter = 0;
     Orthanc::Logging::SetCurrentThreadName(std::string("WADO-LOAD-") + boost::lexical_cast<std::string>(threadCounter++));
 
+    LOG(INFO) << "Loader thread has started";
+
     while (true)
     {
-      std::unique_ptr<InstanceToPreload> instanceToPreload(dynamic_cast<InstanceToPreload*>(that->instancesToPreload_.Dequeue(0)));
-      if (instanceToPreload.get() == NULL)  // that's the signal to exit the thread
+      std::unique_ptr<InstanceToPreload> instancesToPreload(dynamic_cast<InstanceToPreload*>(that->instancesToPreload_.Dequeue(0)));
+      if (instancesToPreload.get() == NULL || that->loadersShouldStop_)  // that's the signal to exit the thread
       {
+        LOG(INFO) << "Loader thread has completed";
         return;
       }
       
@@ -508,7 +531,7 @@
 
       try
       {
-        std::unique_ptr<OrthancPlugins::DicomInstance> dicom(that->GetAndTranscodeDicom(instanceToPreload.get()));
+        std::unique_ptr<OrthancPlugins::DicomInstance> dicom(that->GetAndTranscodeDicom(instancesToPreload.get()));
         that->loadedInstances_.Enqueue(new LoadedInstance(dicom.release()));
       }
       catch (Orthanc::OrthancException& e)
@@ -521,7 +544,6 @@
         LOG(ERROR) << "Unknown error while loading instances ";
         that->loadedInstances_.Enqueue(NULL);
       }
-
     }
   }