From 74e3b9fca646c320f17e983679aab5083f60ec06 Mon Sep 17 00:00:00 2001
From: Eduardo Pinho <eduardopinho@bmd-software.com>
Date: Fri, 25 Aug 2023 09:29:41 +0100
Subject: [PATCH] Improve indexer queue worker thread handling (#664)

* Improve DICOM storage indexer thread handling

- define indexer thread with a Runnable
- name indexer thread to indexer-queue-worker
- add logic to take down old indexer worker thread
   - if DICOM storage service is stopped,
     flip `workerShouldExit` flag and interrupt thread

* Add exception handler to indexer queue worker

- for damage control,
  DICOM storage service is disabled automatically,
  which prevents the archive from accepting files
  that will not be indexed

* Tune logging in indexer worker thread

- reduce Interrupted log line to warning level
- tweak messages
---
 .../pt/ua/dicoogle/server/DicomStorage.java   | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/dicoogle/src/main/java/pt/ua/dicoogle/server/DicomStorage.java b/dicoogle/src/main/java/pt/ua/dicoogle/server/DicomStorage.java
index b31908a54..467f8914b 100755
--- a/dicoogle/src/main/java/pt/ua/dicoogle/server/DicomStorage.java
+++ b/dicoogle/src/main/java/pt/ua/dicoogle/server/DicomStorage.java
@@ -80,6 +80,8 @@ public class DicomStorage extends StorageService {
     private BlockingQueue<ImageElement> queue = new PriorityBlockingQueue<>();
     private NetworkApplicationEntity[] naeArr = null;
     private AtomicLong seqNum = new AtomicLong(0L);
+    private volatile boolean workerShouldExit = false;
+
     private static boolean ASYNC_INDEX = Boolean.valueOf(System.getProperty("dicoogle.index.async", "true"));
 
     /**
@@ -366,11 +368,12 @@ static int compareElementsImpl(Set<String> priorityAETs, String thisAETitle, lon
         return Long.compare(thisSeqNumber, otherSeqNumber);
     }
 
-    class Indexer extends Thread {
+    class IndexerQueueWorker implements Runnable {
         public Collection<IndexerInterface> plugins;
 
+        @Override
         public void run() {
-            while (true) {
+            while (!workerShouldExit) {
                 try {
                     // Fetch an element by the queue taking into account the priorities.
                     ImageElement element = queue.take();
@@ -380,15 +383,16 @@ public void run() {
                     else
                         PluginController.getInstance().indexBlocking(exam);
                 } catch (InterruptedException ex) {
-                    LOG.error("Could not take instance to index", ex);
+                    LOG.warn("Indexer queue worker thread interrupted", ex);
                 } catch (Exception ex) {
-                    LOG.error("Unexpected error in storage index actor", ex);
+                    LOG.error("Unexpected error in indexer queue worker", ex);
                 }
             }
+            LOG.debug("Indexer queue worker exiting by request");
         }
     }
 
-    private Indexer indexer = new Indexer();
+    private Thread indexer = new Thread(new IndexerQueueWorker(), "indexer-queue-worker");
 
     /*
      * Start the Storage Service
@@ -397,6 +401,12 @@ public void run() {
     public void start() throws IOException {
         device.startListening(executor);
         indexer.start();
+
+        indexer.setUncaughtExceptionHandler((Thread t, Throwable e) -> {
+            LOG.error("Fatal error in indexer queue worker", e);
+            ControlServices.getInstance().stopStorage();
+            LOG.warn("DICOM storage service was taken down to prevent further errors");
+        });
     }
 
     /**
@@ -404,5 +414,7 @@ public void start() throws IOException {
      */
     public void stop() {
         device.stopListening();
+        workerShouldExit = true;
+        indexer.interrupt();
     }
 }