Fix issue 597 -- entity manager is properly removed even if close operation fails + unit test.

extensions/persist/pom.xml

@@ -44,5 +44,23 @@
       <version>2.0.0</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>1.9.5</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest-library</artifactId>
+      <version>1.3</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest-core</artifactId>
+      <version>1.3</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

extensions/persist/src/com/google/inject/persist/jpa/JpaPersistService.java

@@ -16,6 +16,7 @@
 
 package com.google.inject.persist.jpa;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -84,12 +85,21 @@ public void end() {
       return;
     }
 
-    em.close();
-    entityManager.remove();
+    try {
+      em.close();
+    }
+    finally {
+      entityManager.remove();
+    }
   }
 
   private volatile EntityManagerFactory emFactory;
 
+  @VisibleForTesting
+  synchronized void start(EntityManagerFactory emFactory) {
+    this.emFactory = emFactory;
+  }
+
   public synchronized void start() {
     Preconditions.checkState(null == emFactory, "Persistence service was already initialized.");
 

extensions/persist/test/com/google/inject/persist/jpa/JpaPersistServiceTest.java

@@ -0,0 +1,53 @@
+package com.google.inject.persist.jpa;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Properties;
+
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.spi.PersistenceProvider;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class JpaPersistServiceTest {
+
+  private static final String PERSISTENCE_UNIT_NAME = "test_persistence_unit_name";
+  private static final Properties PERSISTENCE_PROPERTIES = new Properties();
+
+  private final JpaPersistService sut = new JpaPersistService(PERSISTENCE_UNIT_NAME, PERSISTENCE_PROPERTIES);
+  private final PersistenceProvider provider = mock(PersistenceProvider.class);
+  private final EntityManagerFactory factory = mock(EntityManagerFactory.class);
+  private final EntityManager entityManager = mock(EntityManager.class);
+
+  @Before
+  public void setUp() throws Exception {
+    when(provider.createEntityManagerFactory(PERSISTENCE_UNIT_NAME, PERSISTENCE_PROPERTIES)).thenReturn(factory);
+    when(factory.createEntityManager()).thenReturn(entityManager);
+  }
+
+  @Test
+  public void givenErrorOnEntityManagerClose_whenEndIsCalled_thenEntityManagerIsRemoved() {
+    sut.start(factory);
+    sut.begin();
+
+    // arrange an exception on sut.end(), which invokes entityManager.close()
+    doThrow(SimulatedException.class).when(entityManager).close();
+    try {
+      sut.end();
+      fail("Exception expected");
+    }
+    catch (SimulatedException expected) {
+      assertThat(sut.isWorking(), is(false));
+    }
+  }
+
+  private class SimulatedException extends RuntimeException {
+  }
+}