[solr] 02/03: SOLR-16127 Ordered Executor orphans locks (#779)

[md...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mdrob pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git

commit c347fd6b5c594cc4d996a4435d8057e7fd8c66ce
Author: Mike Drob <md...@apache.org>
AuthorDate: Fri Apr 1 15:23:32 2022 -0500

    SOLR-16127 Ordered Executor orphans locks (#779)
    
    (cherry picked from commit 18a837296edee768bc9b8506af2efbf0f9a03b21)
---
 .../java/org/apache/solr/util/OrderedExecutor.java |  9 +++-
 .../org/apache/solr/util/OrderedExecutorTest.java  | 51 ++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java b/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java
index 15c1072e729..923560a8ac6 100644
--- a/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java
+++ b/solr/core/src/java/org/apache/solr/util/OrderedExecutor.java
@@ -103,7 +103,14 @@ public class OrderedExecutor implements Executor {
         // myLock was successfully inserted
       }
       // won the lock
-      sizeSemaphore.acquire();
+      try {
+        sizeSemaphore.acquire();
+      } catch (InterruptedException e) {
+        if (t != null) {
+          map.remove(t).countDown();
+        }
+        throw e;
+      }
     }
 
     public void remove(T t) {
diff --git a/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java b/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java
index 24bb7c3f3a2..01f79978d43 100644
--- a/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java
+++ b/solr/core/src/test/org/apache/solr/util/OrderedExecutorTest.java
@@ -230,4 +230,55 @@ public class OrderedExecutorTest extends SolrTestCase {
   private static class IntBox {
     int value;
   }
+
+  @Test
+  public void testMaxSize() throws InterruptedException {
+    OrderedExecutor orderedExecutor =
+        new OrderedExecutor(1, ExecutorUtil.newMDCAwareCachedThreadPool("single"));
+
+    CountDownLatch isRunning = new CountDownLatch(1);
+    CountDownLatch blockingLatch = new CountDownLatch(1);
+
+    try {
+      orderedExecutor.execute(
+          () -> {
+            // This will aquire and hold the single max size semaphore permit
+            try {
+              isRunning.countDown();
+              blockingLatch.await();
+            } catch (InterruptedException e) {
+              e.printStackTrace();
+            }
+          });
+
+      isRunning.await(2, TimeUnit.SECONDS);
+
+      // Add another task in a background thread so that we can interrupt it
+      // This _should_ be blocked on the first task because there is only one execution slot
+      CountDownLatch taskTwoFinished = new CountDownLatch(1);
+      Thread t = new Thread(() -> orderedExecutor.execute(2, taskTwoFinished::countDown));
+      t.start();
+      // Interrupt the thread now, but it won't throw until it calls acquire()
+      t.interrupt();
+      // It should complete gracefully from here
+      t.join();
+
+      // Release the first thread
+      assertFalse("Did not expect task #2 to complete", taskTwoFinished.await(0, TimeUnit.SECONDS));
+      blockingLatch.countDown();
+
+      // Tasks without a lock can safely execute again
+      orderedExecutor.execute(() -> {});
+
+      // New threads for lock #2 should be able to execute as well
+      t = new Thread(() -> orderedExecutor.execute(2, () -> {}));
+      t.start();
+
+      // This will also get caught by ThreadLeakControl if it fails
+      t.join(TimeUnit.SECONDS.toMillis(2));
+      assertFalse("Task should have completed", t.isAlive());
+    } finally {
+      orderedExecutor.shutdownAndAwaitTermination();
+    }
+  }
 }