Closing a connection with "usageCount > 1" is actually possible.

btm/src/main/java/bitronix/tm/resource/jdbc/JdbcPooledConnection.java

@@ -280,35 +280,42 @@ public Object getConnectionHandle() throws Exception {
 
         // Increment the usage count
         usageCount++;
+        try {
+            // Only transition to State.ACCESSIBLE on the first usage.  If we're not sharing
+            // connections (default behavior) usageCount is always 1 here, so this transition
+            // will always occur (current behavior unchanged).  If we _are_ sharing connections,
+            // and this is _not_ the first usage, it is valid for the state to already be
+            // State.ACCESSIBLE.  Calling setState() with State.ACCESSIBLE when the state is
+            // already State.ACCESSIBLE fails the sanity check in AbstractXAStatefulHolder.
+            // Even if the connection is shared (usageCount > 1), if the state was State.NOT_ACCESSIBLE
+            // we transition back to State.ACCESSIBLE.
+            if (usageCount == 1 || oldState == State.NOT_ACCESSIBLE) {
+                setState(State.ACCESSIBLE);
+            }
 
-        // Only transition to STATE_ACCESSIBLE on the first usage.  If we're not sharing
-        // connections (default behavior) usageCount is always 1 here, so this transition
-        // will always occur (current behavior unchanged).  If we _are_ sharing connections,
-        // and this is _not_ the first usage, it is valid for the state to already be
-        // STATE_ACCESSIBLE.  Calling setState() with STATE_ACCESSIBLE when the state is
-        // already STATE_ACCESSIBLE fails the sanity check in AbstractXAStatefulHolder.
-        // Even if the connection is shared (usageCount > 1), if the state was STATE_NOT_ACCESSIBLE
-        // we transition back to STATE_ACCESSIBLE.
-        if (usageCount == 1 || oldState == State.NOT_ACCESSIBLE) {
-            setState(State.ACCESSIBLE);
-        }
-
-        if (oldState == State.IN_POOL) {
-            if (log.isDebugEnabled()) { log.debug("connection " + xaConnection + " was in state IN_POOL, testing it"); }
-            testConnection(connection);
-            applyIsolationLevel();
-            applyCursorHoldabilty();
-            if (TransactionContextHelper.currentTransaction() == null) {
-                // it is safe to set the auto-commit flag outside of a global transaction
-                applyLocalAutoCommit();
+            if (oldState == State.IN_POOL) {
+                if (log.isDebugEnabled()) { log.debug("connection " + xaConnection + " was in state IN_POOL, testing it"); }
+                testConnection(connection);
+                applyIsolationLevel();
+                applyCursorHoldabilty();
+                if (TransactionContextHelper.currentTransaction() == null) {
+                    // it is safe to set the auto-commit flag outside of a global transaction
+                    applyLocalAutoCommit();
+                }
+            }
+            else {
+                if (log.isDebugEnabled()) { log.debug("connection " + xaConnection + " was in state " + oldState + ", no need to test it"); }
             }
-        }
-        else {
-            if (log.isDebugEnabled()) { log.debug("connection " + xaConnection + " was in state " + oldState + ", no need to test it"); }
-        }
 
-        if (log.isDebugEnabled()) { log.debug("got connection handle from " + this); }
-        return getConnectionHandle(connection);
+            if (log.isDebugEnabled()) { log.debug("got connection handle from " + this); }
+            return getConnectionHandle(connection);
+        } catch (Exception e) {
+            // This connection must be invalid, so make it inaccessible and revert the usage counter.
+            // Note: Closing a handle with usageCount > 0 "should never happen".
+            setState(State.NOT_ACCESSIBLE);
+            --usageCount;
+            throw e;
+        }
     }
 
     @Override

btm/src/main/java/bitronix/tm/resource/jdbc/proxy/ConnectionJavaProxy.java

@@ -64,16 +64,18 @@ void initialize(JdbcPooledConnection jdbcPooledConnection, Connection connection
     }
 
     @Override
-	public String toString() {
+    public String toString() {
         return "a ConnectionJavaProxy of " + jdbcPooledConnection + " on " + delegate;
     }
 
     /* PooledConnectionProxy interface methods */
 
+    @Override
     public JdbcPooledConnection getPooledConnection() {
         return jdbcPooledConnection;
     }
 
+    @Override
     public Connection getProxiedDelegate() {
         return delegate;
     }
@@ -383,9 +385,9 @@ private void enlistResource() throws SQLException {
             try {
                 TransactionContextHelper.enlistInCurrentTransaction(jdbcPooledConnection);
             } catch (SystemException ex) {
-                throw (SQLException) new SQLException("error enlisting " + this).initCause(ex);
+                throw new SQLException("error enlisting " + this, ex);
             } catch (RollbackException ex) {
-                throw (SQLException) new SQLException("error enlisting " + this).initCause(ex);
+                throw new SQLException("error enlisting " + this, ex);
             }
         } // if getAutomaticEnlistingEnabled
     }

btm/src/main/java/bitronix/tm/resource/jdbc/proxy/LrcXAConnectionJavaProxy.java

@@ -75,8 +75,7 @@ public void removeConnectionEventListener(ConnectionEventListener listener) {
 
     private void fireCloseEvent() {
         if (log.isDebugEnabled()) { log.debug("notifying " + connectionEventListeners.size() + " connectionEventListeners(s) about closing of " + this); }
-        for (int i = 0; i < connectionEventListeners.size(); i++) {
-            ConnectionEventListener connectionEventListener = connectionEventListeners.get(i);
+        for (ConnectionEventListener connectionEventListener : connectionEventListeners) {
             connectionEventListener.connectionClosed(new ConnectionEvent((PooledConnection) delegate));
         }
     }

btm/src/main/java/bitronix/tm/resource/jdbc/proxy/PreparedStatementJavaProxy.java

@@ -51,6 +51,7 @@ void initialize(JdbcPooledConnection jdbcPooledConnection, PreparedStatement sta
         this.pretendClosed = false;
     }
 
+    @Override
     public String toString() {
         return "a PreparedStatementJavaProxy wrapping [" + delegate + "]";
     }

btm/src/main/java/bitronix/tm/resource/jms/DualSessionWrapper.java

@@ -249,7 +249,7 @@ else if (newState == State.CLOSED) {
             }
             messageConsumers.clear();
 
-        } // if newState == STATE_CLOSED
+        } // if newState == State.CLOSED
     }
 
     @Override

btm/src/main/java/bitronix/tm/resource/jms/JmsPooledConnection.java

@@ -246,7 +246,7 @@ public Collection<String> getTransactionGtridsCurrentlyHoldingThis() {
 
     /**
      * {@link JmsPooledConnection} {@link bitronix.tm.resource.common.StateChangeListener}.
-     * When state changes to STATE_CLOSED, the conenction is unregistered from
+     * When state changes to State.CLOSED, the connection is unregistered from
      * {@link bitronix.tm.utils.ManagementRegistrar}.
      */
     private final class JmsPooledConnectionStateChangeListener implements StateChangeListener {
@@ -271,7 +271,7 @@ public void stateChanging(XAStatefulHolder source, State currentState, State fut
 
     /**
      * {@link JmsConnectionHandle} {@link bitronix.tm.resource.common.StateChangeListener}.
-     * When state changes to STATE_CLOSED, the session is removed from the list of opened sessions.
+     * When state changes to State.CLOSED, the session is removed from the list of opened sessions.
      */
     private final class JmsConnectionHandleStateChangeListener implements StateChangeListener {
         @Override

btm/src/main/java/bitronix/tm/timer/TaskScheduler.java

@@ -103,6 +103,7 @@ public int countTasksQueued() {
         }
     }
 
+    @Override
     public void shutdown() {
         boolean wasActive = setActive(false);
 

btm/src/test/java/bitronix/tm/ExceptionAnalyzerTest.java

@@ -40,13 +40,15 @@ public void testExceptionAnalyzer() throws Exception {
         TransactionManagerServices.getConfiguration().setExceptionAnalyzer(TestExceptionAnalyzer.class.getName());
         assertEquals(TestExceptionAnalyzer.class, TransactionManagerServices.getExceptionAnalyzer().getClass());
     }
-    
+
     public static class TestExceptionAnalyzer implements ExceptionAnalyzer {
 
+        @Override
         public String extractExtraXAExceptionDetails(XAException ex) {
             return "";
         }
 
+        @Override
         public void shutdown() {
         }
     }

btm/src/test/java/bitronix/tm/JdbcFailedPoolTest.java

@@ -31,15 +31,17 @@
  */
 public class JdbcFailedPoolTest extends TestCase {
 
+    @Override
     protected void setUp() throws Exception {
         TransactionManagerServices.getJournal().open();
         TransactionManagerServices.getTaskScheduler();
     }
 
+    @Override
     protected void tearDown() throws Exception {
         TransactionManagerServices.getJournal().close();
         TransactionManagerServices.getTaskScheduler().shutdown();
-        
+
         MockitoXADataSource.setStaticGetXAConnectionException(null);
     }
 

btm/src/test/java/bitronix/tm/RestartTest.java

@@ -29,11 +29,11 @@
  */
 public class RestartTest extends TestCase {
 
-
+    @Override
     protected void setUp() throws Exception {
-        Iterator it = ResourceRegistrar.getResourcesUniqueNames().iterator();
+        Iterator<String> it = ResourceRegistrar.getResourcesUniqueNames().iterator();
         while (it.hasNext()) {
-            String name = (String) it.next();
+            String name = it.next();
             ResourceRegistrar.unregister(ResourceRegistrar.get(name));
         }
     }

btm/src/test/java/bitronix/tm/mock/JdbcPoolTest.java

@@ -46,6 +46,7 @@ public class JdbcPoolTest extends TestCase {
     private final static Logger log = LoggerFactory.getLogger(JdbcPoolTest.class);
     private PoolingDataSource pds;
 
+    @Override
     protected void setUp() throws Exception {
         TransactionManagerServices.getConfiguration().setJournal("null").setGracefulShutdownInterval(2);
         TransactionManagerServices.getTransactionManager();
@@ -64,11 +65,11 @@ protected void setUp() throws Exception {
         pds.init();
     }
 
-
+    @Override
     protected void tearDown() throws Exception {
         pds.close();
 
-        TransactionManagerServices.getTransactionManager().shutdown();        
+        TransactionManagerServices.getTransactionManager().shutdown();
     }
 
     public void testObjectProperties() throws Exception {
@@ -186,7 +187,7 @@ public void testPoolShrink() throws Exception {
         TransactionManagerServices.getTaskScheduler().interrupt(); // wake up the task scheduler
         Thread.sleep(1200); // leave enough time for the scheduled shrinking task to do its work
 
-        if (log.isDebugEnabled()) { log.debug("*** checking pool sizes"); }        
+        if (log.isDebugEnabled()) { log.debug("*** checking pool sizes"); }
         assertEquals(1, pool.inPoolSize());
         assertEquals(1, pool.totalPoolSize());
     }
@@ -395,7 +396,7 @@ public void testPoolNotStartingTransactionManager() throws Exception {
         if (log.isDebugEnabled()) { log.debug("*** Starting testPoolNotStartingTransactionManager"); }
         // make sure TM is not running
         TransactionManagerServices.getTransactionManager().shutdown();
-        
+
         PoolingDataSource pds = new PoolingDataSource();
         pds.setMinPoolSize(1);
         pds.setMaxPoolSize(2);

btm/src/test/java/bitronix/tm/mock/JmsPoolTest.java

@@ -37,6 +37,7 @@ public class JmsPoolTest extends TestCase {
 
     private PoolingConnectionFactory pcf;
 
+    @Override
     protected void setUp() throws Exception {
         TransactionManagerServices.getConfiguration().setJournal("null").setGracefulShutdownInterval(2);
         TransactionManagerServices.getTransactionManager();
@@ -55,7 +56,7 @@ protected void setUp() throws Exception {
         pcf.init();
     }
 
-
+    @Override
     protected void tearDown() throws Exception {
         pcf.close();
 
@@ -248,5 +249,5 @@ public void testPoolResetErrorHandling() throws Exception {
         pcf.reset();
         assertEquals(1, pool.inPoolSize());
     }
-    
+
 }

btm/src/test/java/bitronix/tm/mock/NewJdbcProperUsageMockTest.java

@@ -1044,15 +1044,16 @@ static class LooseTransactionThread extends Thread {
         static int successes = 0;
         static int failures = 0;
 
-        private int number;
-        private PoolingDataSource poolingDataSource;
+        private final int number;
+        private final PoolingDataSource poolingDataSource;
         private boolean succesful = false;
 
         public LooseTransactionThread(int number, PoolingDataSource poolingDataSource) {
             this.number = number;
             this.poolingDataSource = poolingDataSource;
         }
 
+        @Override
         public void run() {
             try {
                 UserTransaction ut = TransactionManagerServices.getTransactionManager();

btm/src/test/java/bitronix/tm/mock/NewJdbcStrangeUsageMockTest.java

@@ -19,6 +19,7 @@
 import bitronix.tm.TransactionManagerServices;
 import bitronix.tm.mock.events.ConnectionDequeuedEvent;
 import bitronix.tm.mock.events.ConnectionQueuedEvent;
+import bitronix.tm.mock.events.Event;
 import bitronix.tm.mock.events.EventRecorder;
 import bitronix.tm.mock.events.JournalLogEvent;
 import bitronix.tm.mock.events.XAResourceCommitEvent;
@@ -301,7 +302,7 @@ public void testClosingSuspendedConnectionsInDifferentContext() throws Exception
         assertEquals(POOL_SIZE, pool1.inPoolSize());
 
         // check flow
-        List orderedEvents = EventRecorder.getOrderedEvents();
+        List<? extends Event> orderedEvents = EventRecorder.getOrderedEvents();
         log.info(EventRecorder.dumpToString());
 
         assertEquals(18, orderedEvents.size());

btm/src/test/java/bitronix/tm/mock/events/Event.java

@@ -21,10 +21,10 @@
  */
 public abstract class Event {
 
-    private Exception callStack;
-    private Object source;
-    private Exception exception;
-    private long timestamp;
+    private final Exception callStack;
+    private final Object source;
+    private final Exception exception;
+    private final long timestamp;
 
     protected Event(Object source, Exception ex) {
         this.callStack = new Exception();

btm/src/test/java/bitronix/tm/mock/events/EventRecorder.java

@@ -42,12 +42,12 @@ public static Map<Object, EventRecorder> getEventRecorders() {
         return eventRecorders;
     }
 
-    public static Iterator<Event> iterateEvents() {
+    public static Iterator<? extends Event> iterateEvents() {
         return new EventsIterator(eventRecorders);
     }
 
-    public static List<Event> getOrderedEvents() {
-        Iterator<Event> iterator = iterateEvents();
+    public static List<? extends Event> getOrderedEvents() {
+        Iterator<? extends Event> iterator = iterateEvents();
         List<Event> orderedEvents = new ArrayList<Event>();
         while (iterator.hasNext()) {
             Event ev = iterator.next();
@@ -60,7 +60,7 @@ public static String dumpToString() {
         StringBuilder sb = new StringBuilder();
 
         int i = 0;
-        Iterator<Event> it = iterateEvents();
+        Iterator<? extends Event> it = iterateEvents();
         while (it.hasNext()) {
             Event event = it.next();
             sb.append(i++);

btm/src/test/java/bitronix/tm/twopc/Phase2FailureTest.java

@@ -98,10 +98,8 @@ public void testExpectNoHeuristic() throws Exception {
         int journalCommittingEventCount = 0;
         int journalCommittedEventCount = 0;
         int commitEventCount = 0;
-        List events = EventRecorder.getOrderedEvents();
-        for (int i = 0; i < events.size(); i++) {
-            Event event = (Event) events.get(i);
-
+        List<? extends Event> events = EventRecorder.getOrderedEvents();
+        for (Event event : events) {
             if (event instanceof XAResourceCommitEvent)
                 commitEventCount++;
 
@@ -168,10 +166,8 @@ public void testHeuristicCommit() throws Exception {
         int journalCommittedEventCount = 0;
         int commitEventCount = 0;
         int forgetEventCount = 0;
-        List events = EventRecorder.getOrderedEvents();
-        for (int i = 0; i < events.size(); i++) {
-            Event event = (Event) events.get(i);
-
+        List<? extends Event> events = EventRecorder.getOrderedEvents();
+        for (Event event : events) {
             if (event instanceof XAResourceCommitEvent)
                 commitEventCount++;
 
@@ -235,10 +231,8 @@ public void testHeuristicMixed() throws Exception {
 
         int journalUnknownEventCount = 0;
         int commitEventCount = 0;
-        List events = EventRecorder.getOrderedEvents();
-        for (int i = 0; i < events.size(); i++) {
-            Event event = (Event) events.get(i);
-
+        List<? extends Event> events = EventRecorder.getOrderedEvents();
+        for (Event event : events) {
             if (event instanceof XAResourceCommitEvent)
                 commitEventCount++;
 
@@ -253,9 +247,9 @@ public void testHeuristicMixed() throws Exception {
 
     @Override
     protected void setUp() throws Exception {
-        Iterator it = ResourceRegistrar.getResourcesUniqueNames().iterator();
+        Iterator<String> it = ResourceRegistrar.getResourcesUniqueNames().iterator();
         while (it.hasNext()) {
-            String name = (String) it.next();
+            String name = it.next();
             ResourceRegistrar.unregister(ResourceRegistrar.get(name));
         }