Merge pull request #6175 from joewiz/bugfix/transaction-test-thread-leak

Fix ExecutorService thread leak in TransactionTestDSL

Author: duncdrum

exist-core/src/main/java/org/exist/test/TransactionTestDSL.java

@@ -493,30 +493,29 @@ public Tuple2<U1, U2> execute(final BrokerPool brokerPool, final ExecutionListen
 
             final ThreadGroup transactionsThreadGroup = newInstanceSubThreadGroup(brokerPool, "transactionTestDSL");
 
-            // submit t1
-            final ExecutorService t1ExecutorService = Executors.newSingleThreadExecutor(r -> new Thread(transactionsThreadGroup, r, nameInstanceThread(brokerPool, "transaction-test-dsl.transaction-1-schedule")));
-            final Future<U1> t1Result = t1ExecutorService.submit(() -> {
-                try (final DBBroker broker = brokerPool.get(Optional.of(brokerPool.getSecurityManager().getSystemSubject()));
-                     final Txn txn = brokerPool.getTransactionManager().beginTransaction()) {
-                    final U1 result = lastOperation.t1_state.apply(broker, txn, executionListener, null);
-                    txn.commit();
-                    return result;
-                }
-            });
-
-            // submit t2
-            final ExecutorService t2ExecutorService = Executors.newSingleThreadExecutor(r -> new Thread(transactionsThreadGroup, r, nameInstanceThread(brokerPool, "transaction-test-dsl.transaction-2-schedule")));
-            final Future<U2> t2Result = t2ExecutorService.submit(() -> {
-                try (final DBBroker broker = brokerPool.get(Optional.of(brokerPool.getSecurityManager().getSystemSubject()));
-                     final Txn txn = brokerPool.getTransactionManager().beginTransaction()) {
-                    final U2 result = lastOperation.t2_state.apply(broker, txn, executionListener, null);
-                    txn.commit();
-                    return result;
-                }
-            });
+            // submit t1 and t2 — use try-with-resources to ensure executor shutdown (Java 19+)
+            try (final ExecutorService t1ExecutorService = Executors.newSingleThreadExecutor(r -> new Thread(transactionsThreadGroup, r, nameInstanceThread(brokerPool, "transaction-test-dsl.transaction-1-schedule")));
+                 final ExecutorService t2ExecutorService = Executors.newSingleThreadExecutor(r -> new Thread(transactionsThreadGroup, r, nameInstanceThread(brokerPool, "transaction-test-dsl.transaction-2-schedule")))) {
+
+                final Future<U1> t1Result = t1ExecutorService.submit(() -> {
+                    try (final DBBroker broker = brokerPool.get(Optional.of(brokerPool.getSecurityManager().getSystemSubject()));
+                         final Txn txn = brokerPool.getTransactionManager().beginTransaction()) {
+                        final U1 result = lastOperation.t1_state.apply(broker, txn, executionListener, null);
+                        txn.commit();
+                        return result;
+                    }
+                });
 
-            try {
+                final Future<U2> t2Result = t2ExecutorService.submit(() -> {
+                    try (final DBBroker broker = brokerPool.get(Optional.of(brokerPool.getSecurityManager().getSystemSubject()));
+                         final Txn txn = brokerPool.getTransactionManager().beginTransaction()) {
+                        final U2 result = lastOperation.t2_state.apply(broker, txn, executionListener, null);
+                        txn.commit();
+                        return result;
+                    }
+                });
 
+                //TODO(AR) rather than working with exceptions from Future.get(), it would be better to encapsulate them in a similar way to working on an empty sequence, e.g. could use Either<L,R>???
                 U1 u1 = null;
                 U2 u2 = null;
                 while (true) {
@@ -534,17 +533,6 @@ public Tuple2<U1, U2> execute(final BrokerPool brokerPool, final ExecutionListen
 
                     Thread.sleep(50);
                 }
-            } catch (final ExecutionException | InterruptedException e) {
-                // if we get to here then t1Result or t2Result has thrown an exception
-
-                // force shutdown of transaction threads
-
-                t2ExecutorService.shutdownNow();
-                t1ExecutorService.shutdownNow();
-
-                //TODO(AR) rather than working with exceptions, it would be better to encapsulate them in a similar way to working on an empty sequence, e.g. could use Either<L,R>???
-
-                throw e;
             }
         }
     }