This is an automated email from the ASF dual-hosted git repository. krisden pushed a commit to branch branch_9_0 in repository https://gitbox.apache.org/repos/asf/solr.git
commit 1b48279e07cc86f0eff2a58cf33e86fa3c1d4ebe Author: Mike Drob <[email protected]> AuthorDate: Fri Apr 1 15:23:32 2022 -0500 SOLR-16127 Ordered Executor orphans locks (#779) --- .../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(); + } + } }
