From c839fe6520ccd6e944aacd449e806fa09df3f2de Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Sat, 18 May 2019 11:21:11 +1200 Subject: [PATCH] Adjustment on #1711, more explicit clearing of inactive transactions from thread local --- .../java/io/ebeaninternal/api/SpiEbeanServer.java | 5 +++++ .../io/ebeaninternal/server/core/BeanRequest.java | 9 +++++++++ .../io/ebeaninternal/server/core/DefaultServer.java | 5 +++++ .../server/persist/DefaultPersister.java | 8 ++++++++ .../transaction/DefaultTransactionScopeManager.java | 4 ++-- .../transaction/DefaultTransactionThreadLocal.java | 11 +++++------ .../server/transaction/JdbcTransaction.java | 3 --- .../server/transaction/TransactionManager.java | 7 +++++++ .../server/transaction/TransactionScopeManager.java | 4 ++-- .../java/io/ebeaninternal/api/TDSpiEbeanServer.java | 5 +++++ 10 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java b/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java index f1936cf94..b8e4e9b7c 100644 --- a/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java +++ b/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java @@ -133,6 +133,11 @@ public interface SpiEbeanServer extends ExtendedServer, EbeanServer, BeanLoader, */ void externalModification(TransactionEventTable event); + /** + * Clear an implicit transaction from the scope. + */ + void clearServerTransaction(); + /** * Begin a managed transaction (Uses scope manager / ThreadLocal). */ diff --git a/src/main/java/io/ebeaninternal/server/core/BeanRequest.java b/src/main/java/io/ebeaninternal/server/core/BeanRequest.java index f3a3e5d2a..74327b5f2 100644 --- a/src/main/java/io/ebeaninternal/server/core/BeanRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/BeanRequest.java @@ -78,6 +78,15 @@ public abstract class BeanRequest { } } + /** + * Clear the transaction from the thread local for implicit transactions. + */ + public void clearTransIfRequired() { + if (createdTransaction) { + ebeanServer.clearServerTransaction(); + } + } + /** * Return the server processing the request. Made available for * BeanController and BeanFinder. diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 9d8e129c5..0139c9b57 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -2254,6 +2254,11 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { return new ObtainedTransactionImplicit(trans, this); } + @Override + public void clearServerTransaction() { + transactionManager.clearServerTransaction(); + } + @Override public SpiTransaction beginServerTransaction() { return transactionManager.beginServerTransaction(); diff --git a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index 973bd04c0..0d02a49ef 100644 --- a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -123,6 +123,8 @@ public final class DefaultPersister implements Persister { } catch (RuntimeException e) { request.rollbackTransIfRequired(); throw e; + } finally { + request.clearTransIfRequired(); } } @@ -435,6 +437,8 @@ public final class DefaultPersister implements Persister { } catch (RuntimeException ex) { req.rollbackTransIfRequired(); throw ex; + } finally { + req.clearTransIfRequired(); } } @@ -471,6 +475,8 @@ public final class DefaultPersister implements Persister { } catch (RuntimeException ex) { req.rollbackTransIfRequired(); throw ex; + } finally { + req.clearTransIfRequired(); } } @@ -623,6 +629,8 @@ public final class DefaultPersister implements Persister { } catch (RuntimeException ex) { req.rollbackTransIfRequired(); throw ex; + } finally { + req.clearTransIfRequired(); } } diff --git a/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionScopeManager.java b/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionScopeManager.java index 103d75be6..4a4ca8214 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionScopeManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionScopeManager.java @@ -43,8 +43,8 @@ public class DefaultTransactionScopeManager extends TransactionScopeManager { } @Override - public void clear(SpiTransaction trans) { - DefaultTransactionThreadLocal.clear(serverName, trans); + public void clear() { + DefaultTransactionThreadLocal.clear(serverName); } } diff --git a/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionThreadLocal.java b/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionThreadLocal.java index cecd08671..c73cd9317 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionThreadLocal.java +++ b/src/main/java/io/ebeaninternal/server/transaction/DefaultTransactionThreadLocal.java @@ -41,13 +41,12 @@ public final class DefaultTransactionThreadLocal { } /** - * Clears a transaction from the ThreadLocal to prevent memory leaks. - * Will only clear, if trans == currentTransaction + * Clear a transaction. It should be inactive. */ - public static void clear(String serverName, SpiTransaction trans) { - Map map = local.get(); - if (map.get(serverName) == trans) { - map.remove(serverName); + public static void clear(String serverName) { + SpiTransaction transaction = local.get().remove(serverName); + if (transaction != null && transaction.isActive()) { + throw new IllegalStateException("Clearing an ACTIVE transaction " + transaction); } } diff --git a/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java b/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java index bec80e982..eeb5bbf5b 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java @@ -920,9 +920,6 @@ public class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { } connection = null; active = false; - if (manager != null) { - manager.scope().clear(this); - } profileEnd(); } diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java index 3a79d43b9..e446a9477 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionManager.java @@ -567,6 +567,13 @@ public class TransactionManager implements SpiTransactionManager { } } + /** + * Clear an implicit transaction from thread local scope. + */ + public void clearServerTransaction() { + scopeManager.clear(); + } + /** * Begin an implicit transaction. */ diff --git a/src/main/java/io/ebeaninternal/server/transaction/TransactionScopeManager.java b/src/main/java/io/ebeaninternal/server/transaction/TransactionScopeManager.java index 66ae60f33..98c192a6c 100644 --- a/src/main/java/io/ebeaninternal/server/transaction/TransactionScopeManager.java +++ b/src/main/java/io/ebeaninternal/server/transaction/TransactionScopeManager.java @@ -35,9 +35,9 @@ public abstract class TransactionScopeManager implements SpiTransactionScopeMana public abstract void set(SpiTransaction trans); /** - * Clears the given Transaction for this serverName and Thread. + * Clears the current Transaction from thread local scope (for implicit transactions). */ - public abstract void clear(SpiTransaction trans); + public abstract void clear(); /** * Replace the current transaction with this one. diff --git a/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java b/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java index 44bad8903..bde5e8b34 100644 --- a/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java +++ b/src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java @@ -227,6 +227,11 @@ public class TDSpiEbeanServer implements SpiEbeanServer { } + @Override + public void clearServerTransaction() { + + } + @Override public SpiTransaction beginServerTransaction() { return null;