From 196da18009dda9e943387210370836e9837ddb95 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 17 Nov 2025 15:21:21 -0800 Subject: [PATCH 1/2] Avoid leaving connection in bad state after exception during precommit task --- api/src/org/labkey/api/data/DbScope.java | 31 +++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 70f28bece57..502ff95c684 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -26,6 +26,7 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.action.ApiUsageException; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.cache.Cache; import org.labkey.api.data.ConnectionWrapper.Closer; @@ -2166,8 +2167,7 @@ public static void closeAllConnectionsForCurrentThread() } try { - LOG.warn("Forcing close of still-pending transaction object. Current stack is ", new Throwable()); - LOG.warn("Forcing close of still-pending transaction object started at ", t._creation); + LOG.warn("Forcing close of still-pending transaction object started at {}. Current stack is ", t._creation, new Throwable()); t.close(); } catch (ConnectionAlreadyReleasedException ignored) @@ -2715,16 +2715,17 @@ public void commit() conn.commit(); conn.setAutoCommit(true); LOG.debug("setAutoCommit(true)"); - if (null != _closeOnClose) - try { _closeOnClose.close(); } catch (Exception ignore) {} } finally { + if (null != _closeOnClose) + try { _closeOnClose.close(); } catch (Exception ignore) {} if (null != conn) conn.internalClose(); - } - popCurrentTransaction(); + // Make sure to pop whether we successfully committed or not + popCurrentTransaction(); + } CommitTaskOption.POSTCOMMIT.run(this); } @@ -3164,6 +3165,24 @@ public void testAutoCommitFailure() closeAllConnectionsForCurrentThread(); } + @Test + public void tesCommitTaskFailure() + { + String message = "Expected failure"; + try (Transaction t = getLabKeyScope().ensureTransaction()) + { + t.addCommitTask(() -> { throw new ApiUsageException("Expected failure"); }, CommitTaskOption.PRECOMMIT); + t.commit(); + fail("Shouldn't have gotten here, expected ApiUsageException"); + } + catch (ApiUsageException e) + { + assertEquals("Bad message", message, e.getMessage()); + } + assertFalse(getLabKeyScope().isTransactionActive()); + closeAllConnectionsForCurrentThread(); + } + @Test public void testLockReleasedException() { From 58d10a06e8f98482825743ce959afc57a0631dac Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Wed, 19 Nov 2025 13:46:04 -0800 Subject: [PATCH 2/2] Ensure caches are closed regardless of success --- api/src/org/labkey/api/data/DbScope.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 502ff95c684..afe69fbff21 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -2385,12 +2385,17 @@ public void run(TransactionImpl transaction) // Copy to avoid ConcurrentModificationExceptions, need to retain original order from LinkedHashMap List tasks = new ArrayList<>(getRunnables(transaction).keySet()); - for (Runnable task : tasks) + try { - task.run(); + for (Runnable task : tasks) + { + task.run(); + } + } + finally + { + transaction.closeCaches(); } - - transaction.closeCaches(); } public T add(TransactionImpl transaction, T task)