You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by ti...@apache.org on 2010/02/08 10:28:36 UTC

svn commit: r907590 - in /incubator/aries/trunk/jpa/jpa-container-context/src: main/java/org/apache/aries/jpa/container/context/transaction/impl/ test/java/org/apache/aries/jpa/container/context/transaction/impl/

Author: timothyjward
Date: Mon Feb  8 09:28:35 2010
New Revision: 907590

URL: http://svn.apache.org/viewvc?rev=907590&view=rev
Log:
ARIES-140 : Ensure EntityManagers are properly cleaned up

Modified:
    incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java
    incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java

Modified: incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java
URL: http://svn.apache.org/viewvc/incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java?rev=907590&r1=907589&r2=907590&view=diff
==============================================================================
--- incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java (original)
+++ incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java Mon Feb  8 09:28:35 2010
@@ -20,8 +20,6 @@
 
 import java.util.IdentityHashMap;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
 
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
@@ -35,9 +33,28 @@
 /**
  * This class is used to manage the lifecycle of JTA peristence contexts
  */
-public class JTAPersistenceContextRegistry {
+public final class JTAPersistenceContextRegistry {
   /** Logger */
   private static final Logger _logger = LoggerFactory.getLogger("org.apache.aries.jpa.container.context");
+  /** The unique key we use to find our Map */
+  private static final TSRKey EMF_MAP_KEY = new TSRKey();
+  
+  /** 
+   * A simple class to avoid key collisions in the TransactionSynchronizationRegistry. 
+   * As recommended by {@link TransactionSynchronizationRegistry#putResource(Object, Object)}
+   */
+  private final static class TSRKey {
+
+    @Override
+    public final boolean equals(Object o) {
+      return (this == o);
+    }
+
+    @Override
+    public final int hashCode() {
+      return 0xDEADBEEF;
+    }
+  }
   
   /** 
    * The transaction synchronization registry, used to determine the currently
@@ -46,14 +63,6 @@
   private TransactionSynchronizationRegistry tranRegistry;
 
   /**
-   * The registry of active persistence contexts. The outer map must be thread safe, as
-   * multiple threads can request persistence contexts. The inner Map does not need to
-   * be thread safe as only one thread can be in a transaction. As a result the inner
-   * Map will never be accessed concurrently.
-   */
-  private final ConcurrentMap<Object, Map<EntityManagerFactory, EntityManager>> persistenceContextRegistry = new ConcurrentHashMap<Object, Map<EntityManagerFactory,EntityManager>>();
-  
-  /**
    * Get a PersistenceContext for the current transaction. The persistence context will 
    * automatically be closed when the transaction completes.
    * 
@@ -65,44 +74,52 @@
    *         need to be wrappered to obey the JPA spec by throwing the correct exceptions
    * @throws {@link TransactionRequiredException} if there is no active transaction.
    */
-  public EntityManager getCurrentPersistenceContext(EntityManagerFactory persistenceUnit, Map<?,?> properties) throws TransactionRequiredException
+  @SuppressWarnings("unchecked")
+  public final EntityManager getCurrentPersistenceContext(EntityManagerFactory persistenceUnit, Map<?,?> properties) throws TransactionRequiredException
   {
     //There will only ever be one thread associated with a transaction at a given time
     //As a result, it is only the outer map that needs to be thread safe.
     
-    Object transactionKey = tranRegistry.getTransactionKey();
-    
     //Throw the error on to the client
-    if(transactionKey == null) {
+    if(!!!isTransactionActive()) {
       throw new TransactionRequiredException("No transaction currently active");
     }
+    EntityManager toReturn = null;
     
     //Get hold of the Map. If there is no Map already registered then add one.
     //We don't need to worry about a race condition, as no other thread will
-    //share our transaction
-    Map<EntityManagerFactory, EntityManager> contextsForTransaction = persistenceContextRegistry.get(transactionKey);
+    //share our transaction and be able to access our Map
+    Map<EntityManagerFactory, EntityManager> contextsForTransaction = (Map<EntityManagerFactory, EntityManager>) tranRegistry.getResource(EMF_MAP_KEY);
     
-    //If we need to, create a new Map add it to the registry, and register it for cleanup
-    if(contextsForTransaction == null) {
+    //If we have a map then find an EntityManager, else create a new Map add it to the registry, and register for cleanup
+    if(contextsForTransaction != null) {
+      toReturn = contextsForTransaction.get(persistenceUnit);
+    } else {
       contextsForTransaction = new IdentityHashMap<EntityManagerFactory, EntityManager>();
-      persistenceContextRegistry.put(transactionKey, contextsForTransaction);
       try {
-        tranRegistry.registerInterposedSynchronization(new EntityManagerClearUp(transactionKey));
+        tranRegistry.putResource(EMF_MAP_KEY, contextsForTransaction);
       } catch (IllegalStateException e) {
-        persistenceContextRegistry.remove(transactionKey);
-        throw new TransactionRequiredException("Unable to synchronize with transaction " + transactionKey);
+        _logger.warn("Unable to create a persistence context for the transaction {} because the is not active", new Object[] {tranRegistry.getTransactionKey()});
+        throw new TransactionRequiredException("Unable to assiociate resources with transaction " + tranRegistry.getTransactionKey());
       }
     }
     
-    //Still only one thread for this transaction, so don't worry about any race conditions
-    EntityManager toReturn = contextsForTransaction.get(persistenceUnit);
-    
+    //If we have no previously created EntityManager
     if(toReturn == null) {
       toReturn = (properties == null) ? persistenceUnit.createEntityManager() : persistenceUnit.createEntityManager(properties);
+      if(_logger.isDebugEnabled())
+        _logger.debug("Created a new persistence context {} for transaction {}.", new Object[] {toReturn, tranRegistry.getTransactionKey()});
+      try {
+        tranRegistry.registerInterposedSynchronization(new EntityManagerClearUp(toReturn));
+      } catch (IllegalStateException e) {
+        _logger.warn("No persistence context could be created as the JPA container could not register a synchronization with the transaction {}.", new Object[] {tranRegistry.getTransactionKey()});
+        toReturn.close();
+        throw new TransactionRequiredException("Unable to synchronize with transaction " + tranRegistry.getTransactionKey());
+      }
       contextsForTransaction.put(persistenceUnit, toReturn);
     } else {
       if(_logger.isDebugEnabled())
-        _logger.debug("Re-using a persistence context for transaction " + transactionKey);
+        _logger.debug("Re-using a persistence context for transaction " + tranRegistry.getTransactionKey());
     }
     return toReturn;
   }
@@ -111,7 +128,7 @@
    * Determine whether there is an active transaction on the thread
    * @return
    */
-  public boolean isTransactionActive()
+  public final boolean isTransactionActive()
   {
     return tranRegistry.getTransactionKey() != null;
   }
@@ -120,7 +137,7 @@
    * Provide a {@link TransactionSynchronizationRegistry} to use
    * @param tranRegistry
    */
-  public void setTranRegistry(TransactionSynchronizationRegistry tranRegistry) {
+  public final void setTranRegistry(TransactionSynchronizationRegistry tranRegistry) {
     this.tranRegistry = tranRegistry;
   }
 
@@ -128,28 +145,31 @@
    * This class is used to close EntityManager instances once the transaction has committed,
    * and clear the persistenceContextRegistry of old persistence contexts.
    */
-  private class EntityManagerClearUp implements Synchronization {
+  private final static class EntityManagerClearUp implements Synchronization {
 
-    private final Object key;
+    private final EntityManager context;
     
-    public EntityManagerClearUp(Object transactionKey) {
-      key = transactionKey;
+    /**
+     * Create a Synchronization to clear up our EntityManagers
+     * @param em
+     */
+    public EntityManagerClearUp(EntityManager em)
+    {
+      context = em;
     }
     
-    public void afterCompletion(int arg0) {
+    public final void beforeCompletion() {
       //This is a no-op;
     }
 
-    public void beforeCompletion() {
-      Map<EntityManagerFactory, EntityManager> tidyUp = persistenceContextRegistry.remove(key);
-      if(tidyUp != null) {
-        for(EntityManager em : tidyUp.values()) {
-          try {
-            em.close();
-          } catch (Exception e) {
-            _logger.warn("There was an error when the container closed an EntityManager", em);
-          }
-        }
+    @SuppressWarnings("unchecked")
+    public final void afterCompletion(int arg0) {
+      if(_logger.isDebugEnabled())
+        _logger.debug("Clearing up EntityManager {} as the transaction has completed.", new Object[] {context});
+      try {
+        context.close();
+      } catch (Exception e) {
+        _logger.warn("There was an error when the container closed an EntityManager", context);
       }
     }
   }

Modified: incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java
URL: http://svn.apache.org/viewvc/incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java?rev=907590&r1=907589&r2=907590&view=diff
==============================================================================
--- incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java (original)
+++ incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java Mon Feb  8 09:28:35 2010
@@ -21,11 +21,14 @@
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
+import javax.transaction.Status;
 import javax.transaction.Synchronization;
 import javax.transaction.TransactionSynchronizationRegistry;
 
@@ -40,23 +43,52 @@
   {
     private String key;
     
-    private Map<String, Synchronization> syncs = new HashMap<String, Synchronization>();
+    private Map<String, List<Synchronization>> syncs = new HashMap<String, List<Synchronization>>();
+    
+    private Map<String, Map<Object,Object>> resources = new HashMap<String, Map<Object,Object>>();
     
     public void setTransactionKey(String s)
     {
       key = s;
     }
+    
     public Object getTransactionKey() {
       return key;
     }
 
     public void registerInterposedSynchronization(Synchronization arg0) {
-      syncs.put(key, arg0);
+      List<Synchronization> list = syncs.get(key);
+      if(list == null) {
+        list = new ArrayList<Synchronization>();
+        syncs.put(key, list);
+      }
+       list.add(arg0);
+    }
+    
+    public Object getResource(Object o) {
+      Object toReturn = null;
+      Map<Object, Object> map = resources.get(key);
+      if(map != null)
+        toReturn = map.get(o);
+      return toReturn;
+    }
+    
+    public void putResource(Object resourceKey, Object value) {
+      Map<Object, Object> map = resources.get(key);
+      if(map == null) {
+        map = new HashMap<Object, Object>();
+        resources.put(key, map);
+      }
+      map.put(resourceKey, value);
     }
     
-    public void beforeCompletion(String s)
+    
+    public void afterCompletion(String s)
     {
-      syncs.get(s).beforeCompletion();
+      for(Synchronization sync : syncs.get(s))
+        sync.afterCompletion(Status.STATUS_COMMITTED);
+      
+      resources.remove(s);
     }
   }
   
@@ -120,7 +152,7 @@
     Skeleton.getSkeleton(em2a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     assertSame("We should get the same delegate!", em2a, em2b);
     
-    reg.beforeCompletion("");
+    reg.afterCompletion("");
     
     Skeleton.getSkeleton(em1a).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"),1);
     Skeleton.getSkeleton(em2a).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"),1);
@@ -155,15 +187,16 @@
     Skeleton.getSkeleton(em2a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     Skeleton.getSkeleton(em2b).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     
-    
-    reg.beforeCompletion("b");
+    reg.setTransactionKey("b");
+    reg.afterCompletion("b");
     
     Skeleton.getSkeleton(em1a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     Skeleton.getSkeleton(em1b).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);
     Skeleton.getSkeleton(em2a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     Skeleton.getSkeleton(em2b).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);
-
-    reg.beforeCompletion("a");
+    
+    reg.setTransactionKey("a");
+    reg.afterCompletion("a");
     
     Skeleton.getSkeleton(em1a).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);
     Skeleton.getSkeleton(em1b).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);