Author: bcarothers Date: 2010-01-09 19:38:45 -0500 (Sat, 09 Jan 2010) New Revision: 1578 Modified: trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicJpaConnection.java Log: DNA-617 NPE while updating indexes (during integration tests) Attach patch that corrects the behavior of the BasicJpaConnector with regard to EntityManager lifetime. Prior to the patch, the BJC obtained an EntityManager in its constructor and released the EntityManager in its close method. The typical lifecycle for a connection is a call to the constructor followed by a single call to execute(...) followed by a call to close(). With this lifecycle, the EntityManager is only used for one call to execute(...) and the transaction is committed or rolled back after the call as appropriate. Now that the RepositoryConnectionPool is being used (and was fixed to reuse connections), the typical lifecycle is one call to the constructor followed by multiple calls to execute(...) within different transactions followed by (eventually) a call to close(). Since the same EntityManager was being held for the entire life of the BJC object, the EntityManager would be used for multiple transactions without any attempt being made to clear out its cache. We're using Hibernate as our JPA implementation, so the Hibernate session cache was gradually accumulating copies of the data in the database _that were gradually falling out of sync with the database_. In other words, BJC1 would be used to create a node and set a property leaving the ChildEntity and the PropertiesEntity for the node in the Hibernate session cache for BJC1's EntityManager. BJC2 would then be used to read the same node and update its property. Since the entities for the node weren't yet in in the session cache for BJC2's EntityManager, BJC2 would execute select statements to load the necessary data and would perform the updates. THEN in a subsequent call, BJC1 would be reused to read from the same node. Since BJC already had the ChildEntity and PropertiesEntity for the node in its session cache, it would reuse the cached (and now stale) versions instead of trying to hit the database. Hilarity ensued. The patch makes BJC obtain an EntityManager at the beginning of the execute(...) call and release the EntityManager at the end of the execute(...) call. Since "released" EntityManager objects are really closed by the EntityManagers factory object, this will cure the session cache issue. This fixes all but 5 of the TCK failures with the JPA connector using the basic model. Modified: trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicJpaConnection.java =================================================================== --- trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicJpaConnection.java 2010-01-09 22:27:32 UTC (rev 1577) +++ trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicJpaConnection.java 2010-01-10 00:38:45 UTC (rev 1578) @@ -76,8 +76,8 @@ this.name = sourceName; this.cachePolicy = cachePolicy; // may be null this.entityManagers = entityManagers; - this.entityManager = entityManagers.checkout(); - assert this.entityManagers != null; + // this.entityManager = entityManagers.checkout(); + // assert this.entityManagers != null; this.rootNodeUuid = rootNodeUuid; this.largeValueMinimumSizeInBytes = largeValueMinimumSizeInBytes; this.compressData = compressData; @@ -132,9 +132,6 @@ */ public void execute( ExecutionContext context, Request request ) throws RepositorySourceException { - if (entityManager == null) { - throw new RepositorySourceException(JpaConnectorI18n.connectionIsNoLongerOpen.text(name)); - } Logger logger = context.getLogger(getClass()); Stopwatch sw = null; @@ -142,14 +139,21 @@ sw = new Stopwatch(); sw.start(); } - // Do any commands update/write? - RequestProcessor processor = new BasicRequestProcessor(name, context, observer, entityManager, rootNodeUuid, - nameOfDefaultWorkspace, predefinedWorkspaceNames, - largeValueMinimumSizeInBytes, creatingWorkspacesAllowed, - compressData, enforceReferentialIntegrity); + RequestProcessor processor = null; boolean commit = true; + try { + this.entityManager = entityManagers.checkout(); + + if (entityManager == null) { + throw new RepositorySourceException(JpaConnectorI18n.connectionIsNoLongerOpen.text(name)); + } + + // Do any commands update/write? + processor = new BasicRequestProcessor(name, context, observer, entityManager, rootNodeUuid, nameOfDefaultWorkspace, + predefinedWorkspaceNames, largeValueMinimumSizeInBytes, + creatingWorkspacesAllowed, compressData, enforceReferentialIntegrity); // Obtain the lock and execute the commands ... processor.process(request); if (request.hasError() && !request.isReadOnly()) { @@ -184,7 +188,15 @@ processor.notifyObserverOfChanges(); } } + + // Do this only once ... + try { + entityManagers.checkin(entityManager); + } finally { + entityManager = null; + } } + if (logger.isTraceEnabled()) { assert sw != null; sw.stop(); @@ -198,14 +210,6 @@ * @see org.jboss.dna.graph.connector.RepositoryConnection#close() */ public void close() { - if (entityManager != null) { - // Do this only once ... - try { - entityManagers.checkin(entityManager); - } finally { - entityManager = null; - } - } } }