geronimo-scm mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From djen...@apache.org
Subject svn commit: r620093 - in /geronimo/components/txmanager/trunk/geronimo-connector/src: main/java/org/apache/geronimo/connector/outbound/ test/java/org/apache/geronimo/connector/outbound/
Date Sat, 09 Feb 2008 09:16:30 GMT
Author: djencks
Date: Sat Feb  9 01:16:29 2008
New Revision: 620093

URL: http://svn.apache.org/viewvc?rev=620093&view=rev
Log:
GERONIMO-3834 Fix the permit leak when connections can't be created.  Simplify some of the
pool code and add some pool tests for permit count and resizing

Added:
    geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
  (with props)
    geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
  (with props)
    geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
  (with props)
Removed:
    geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/PoolDequeTest.java
Modified:
    geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolConnectionInterceptor.java
    geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/MCFConnectionInterceptor.java
    geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolConnectionInterceptor.java
    geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllConnectionInterceptor.java

Modified: geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolConnectionInterceptor.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolConnectionInterceptor.java?rev=620093&r1=620092&r2=620093&view=diff
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolConnectionInterceptor.java
(original)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolConnectionInterceptor.java
Sat Feb  9 01:16:29 2008
@@ -28,6 +28,7 @@
 import javax.resource.ResourceException;
 import javax.resource.spi.ConnectionRequestInfo;
 import javax.resource.spi.ManagedConnectionFactory;
+import javax.resource.spi.ManagedConnection;
 import javax.security.auth.Subject;
 
 import org.apache.commons.logging.Log;
@@ -75,7 +76,12 @@
             resizeLock.readLock().lock();
             try {
                 if (permits.tryAcquire(blockingTimeoutMilliseconds, TimeUnit.MILLISECONDS))
{
-                    internalGetConnection(connectionInfo);
+                    try {
+                        internalGetConnection(connectionInfo);
+                    } catch (ResourceException e) {
+                        permits.release();
+                        throw e;
+                    }
                 } else {
                     throw new ResourceException("No ManagedConnections available "
                             + "within configured blocking timeout ( "
@@ -120,9 +126,9 @@
                 return;
             }
 
-            boolean wasInPool = internalReturn(connectionInfo, connectionReturnAction);
+            boolean releasePermit = internalReturn(connectionInfo, connectionReturnAction);
 
-            if (!wasInPool) {
+            if (releasePermit) {
                 permits.release();
             }
         } finally {
@@ -130,7 +136,45 @@
         }
     }
 
-    protected abstract boolean internalReturn(ConnectionInfo connectionInfo, ConnectionReturnAction
connectionReturnAction);
+    protected boolean internalReturn(ConnectionInfo connectionInfo, ConnectionReturnAction
connectionReturnAction) {
+        ManagedConnectionInfo mci = connectionInfo.getManagedConnectionInfo();
+        ManagedConnection mc = mci.getManagedConnection();
+        try {
+            mc.cleanup();
+        } catch (ResourceException e) {
+            connectionReturnAction = ConnectionReturnAction.DESTROY;
+        }
+
+        boolean releasePermit;
+        synchronized (getPool()) {
+            // a bit redundant, but this closes a small timing hole...
+            if (destroyed) {
+                try {
+                    mc.destroy();
+                }
+                catch (ResourceException re) {
+                    //ignore
+                }
+                return doRemove(mci);
+            }
+            if (shrinkLater > 0) {
+                //nothing can get in the pool while shrinkLater > 0, so releasePermit
is false here.
+                connectionReturnAction = ConnectionReturnAction.DESTROY;
+                shrinkLater--;
+                releasePermit = false;
+            } else if (connectionReturnAction == ConnectionReturnAction.RETURN_HANDLE) {
+                mci.setLastUsed(System.currentTimeMillis());
+                doAdd(mci);
+                return true;
+            } else {
+                releasePermit = doRemove(mci);
+            }
+        }
+        //we must destroy connection.
+        next.returnConnection(connectionInfo, connectionReturnAction);
+        connectionCount--;
+        return releasePermit;
+    }
 
     protected abstract void internalDestroy();
 
@@ -147,7 +191,9 @@
         return 1;
     }
 
-    public abstract int getPartitionMaxSize();
+    public int getPartitionMaxSize() {
+        return maxSize;
+    }
 
     public void setPartitionMaxSize(int newMaxSize) throws InterruptedException {
         if (newMaxSize <= 0) {
@@ -157,22 +203,30 @@
             resizeLock.writeLock().lock();
             try {
                 ResizeInfo resizeInfo = new ResizeInfo(this.minSize, permits.availablePermits(),
connectionCount, newMaxSize);
-                this.shrinkLater = resizeInfo.getShrinkLater();
-
                 permits = new Semaphore(newMaxSize, true);
                 //pre-acquire permits for the existing checked out connections that will
not be closed when they are returned.
                 for (int i = 0; i < resizeInfo.getTransferCheckedOut(); i++) {
                     permits.acquire();
                 }
+                //make sure shrinkLater is 0 while discarding excess connections
+                this.shrinkLater = 0;
                 //transfer connections we are going to keep
                 transferConnections(newMaxSize, resizeInfo.getShrinkNow());
+                this.shrinkLater = resizeInfo.getShrinkLater();
                 this.minSize = resizeInfo.getNewMinSize();
+                this.maxSize = newMaxSize;
             } finally {
                 resizeLock.writeLock().unlock();
             }
         }
     }
 
+    protected abstract boolean doRemove(ManagedConnectionInfo mci);
+
+    protected abstract void doAdd(ManagedConnectionInfo mci);
+
+    protected abstract Object getPool();
+
 
     static final class ResizeInfo {
 
@@ -271,7 +325,17 @@
 
     protected abstract void getExpiredManagedConnectionInfos(long threshold, List<ManagedConnectionInfo>
killList);
 
-    protected abstract boolean addToPool(ManagedConnectionInfo mci);
+    protected boolean addToPool(ManagedConnectionInfo mci) {
+        boolean added;
+        synchronized (getPool()) {
+            connectionCount++;
+            added = getPartitionMaxSize() > getIdleConnectionCount();
+            if (added) {
+                doAdd(mci);
+            }
+        }
+        return added;
+    }
 
     // static class to permit chain of strong references from preventing ClassLoaders
     // from being GC'ed.

Modified: geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/MCFConnectionInterceptor.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/MCFConnectionInterceptor.java?rev=620093&r1=620092&r2=620093&view=diff
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/MCFConnectionInterceptor.java
(original)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/MCFConnectionInterceptor.java
Sat Feb  9 01:16:29 2008
@@ -45,8 +45,7 @@
         }
         
         try {
-            ManagedConnection mc =
-                mci.getManagedConnectionFactory().createManagedConnection(
+            ManagedConnection mc = mci.getManagedConnectionFactory().createManagedConnection(
                         mci.getSubject(),
                         mci.getConnectionRequestInfo());
             mci.setManagedConnection(mc);

Modified: geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolConnectionInterceptor.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolConnectionInterceptor.java?rev=620093&r1=620092&r2=620093&view=diff
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolConnectionInterceptor.java
(original)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolConnectionInterceptor.java
Sat Feb  9 01:16:29 2008
@@ -17,7 +17,7 @@
 
 package org.apache.geronimo.connector.outbound;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
@@ -43,7 +43,9 @@
 
     private boolean selectOneAssumeMatch;
 
-    private PoolDeque pool;
+    //pool is mutable but only changed when protected by write lock on resizelock in superclass
+//    private PoolDeque pool;
+    private final List<ManagedConnectionInfo> pool;
 
     public SinglePoolConnectionInterceptor(final ConnectionInterceptor next,
                                            int maxSize,
@@ -52,7 +54,8 @@
                                            int idleTimeoutMinutes,
                                            boolean selectOneAssumeMatch) {
         super(next, maxSize, minSize, blockingTimeoutMilliseconds, idleTimeoutMinutes);
-        pool = new PoolDeque(maxSize);
+//        pool = new PoolDeque(maxSize);
+        pool = new ArrayList<ManagedConnectionInfo>(maxSize);
         this.selectOneAssumeMatch = selectOneAssumeMatch;
     }
 
@@ -62,7 +65,7 @@
                 throw new ResourceException("ManagedConnection pool has been destroyed");
             }
 
-            ManagedConnectionInfo newMCI = null;
+            ManagedConnectionInfo newMCI;
             if (pool.isEmpty()) {
                 next.getConnection(connectionInfo);
                 connectionCount++;
@@ -71,7 +74,7 @@
                 }
                 return;
             } else {
-                newMCI = pool.removeLast();
+                newMCI = pool.remove(pool.size() - 1);
             }
             if (connectionCount < minSize) {
                 timer.schedule(new FillTask(connectionInfo), 10);
@@ -85,12 +88,9 @@
             }
             try {
                 ManagedConnectionInfo mci = connectionInfo.getManagedConnectionInfo();
-                ManagedConnection matchedMC =
-                        newMCI
-                        .getManagedConnectionFactory()
-                        .matchManagedConnections(Collections.singleton(newMCI.getManagedConnection()),
-                                mci.getSubject(),
-                                mci.getConnectionRequestInfo());
+                ManagedConnection matchedMC = newMCI.getManagedConnectionFactory().matchManagedConnections(Collections.singleton(newMCI.getManagedConnection()),
+                        mci.getSubject(),
+                        mci.getConnectionRequestInfo());
                 if (matchedMC != null) {
                     connectionInfo.setManagedConnectionInfo(newMCI);
                     if (log.isTraceEnabled()) {
@@ -100,16 +100,14 @@
                     //matching failed.
                     ConnectionInfo returnCI = new ConnectionInfo();
                     returnCI.setManagedConnectionInfo(newMCI);
-                    returnConnection(returnCI,
-                            ConnectionReturnAction.RETURN_HANDLE);
+                    returnConnection(returnCI, ConnectionReturnAction.RETURN_HANDLE);
                     throw new ResourceException("The pooling strategy does not match the
MatchManagedConnections implementation.  Please investigate and reconfigure this pool");
                 }
             } catch (ResourceException e) {
                 //something is wrong: destroy connection, rethrow, release permit
                 ConnectionInfo returnCI = new ConnectionInfo();
                 returnCI.setManagedConnectionInfo(newMCI);
-                returnConnection(returnCI,
-                        ConnectionReturnAction.DESTROY);
+                returnConnection(returnCI, ConnectionReturnAction.DESTROY);
                 throw e;
             }
         }
@@ -118,167 +116,50 @@
     protected void internalDestroy() {
         synchronized (pool) {
             while (!pool.isEmpty()) {
-                ManagedConnection mc = pool.removeLast().getManagedConnection();
+                ManagedConnection mc = pool.remove(pool.size() - 1).getManagedConnection();
                 if (mc != null) {
                     try {
                         mc.destroy();
                     }
-                    catch (ResourceException re) { } // ignore
+                    catch (ResourceException re) {
+                        //ignore
+                    }
                 }
             }
         }
     }
 
-    protected boolean internalReturn(ConnectionInfo connectionInfo, ConnectionReturnAction
connectionReturnAction) {
-        ManagedConnectionInfo mci = connectionInfo.getManagedConnectionInfo();
-        ManagedConnection mc = mci.getManagedConnection();
-        if (connectionReturnAction == ConnectionReturnAction.RETURN_HANDLE) {
-            try {
-                mc.cleanup();
-            } catch (ResourceException e) {
-                connectionReturnAction = ConnectionReturnAction.DESTROY;
-            }
-        }
-        boolean wasInPool = false;
-        synchronized (pool) {
-            // a bit redundant with returnConnection check in AbstractSinglePoolConnectionInterceptor,

-            // but checking here closes a small timing hole...
-            if (destroyed) {
-                try {
-                    mc.destroy();
-                }
-                catch (ResourceException re) { } // ignore
-                return pool.remove(mci);
-            }
+    protected Object getPool() {
+        return pool;
+    }
 
-            if (shrinkLater > 0) {
-                //nothing can get in the pool while shrinkLater > 0, so wasInPool is false
here.
-                connectionReturnAction = ConnectionReturnAction.DESTROY;
-                shrinkLater--;
-            } else if (connectionReturnAction == ConnectionReturnAction.RETURN_HANDLE) {
-                mci.setLastUsed(System.currentTimeMillis());
-                pool.add(mci);
-                return wasInPool;
-            } else {
-                wasInPool = pool.remove(mci);
-            }
-        }
-        //we must destroy connection.
-        next.returnConnection(connectionInfo, connectionReturnAction);
-        connectionCount--;
-        return wasInPool;
+    protected void doAdd(ManagedConnectionInfo mci) {
+        pool.add(mci);
     }
 
-    public int getPartitionMaxSize() {
-        return pool.capacity();
+    protected boolean doRemove(ManagedConnectionInfo mci) {
+        return !pool.remove(mci);
     }
 
     protected void transferConnections(int maxSize, int shrinkNow) {
-        //1st example: copy 0 (none)
-        //2nd example: copy 10 (all)
-        PoolDeque oldPool = pool;
-        pool = new PoolDeque(maxSize);
-        //since we have replaced pool already, pool.remove will be very fast:-)
         for (int i = 0; i < shrinkNow; i++) {
-            ConnectionInfo killInfo = new ConnectionInfo(oldPool.peek(i));
+            ConnectionInfo killInfo = new ConnectionInfo(pool.get(0));
             internalReturn(killInfo, ConnectionReturnAction.DESTROY);
         }
-        for (int i = shrinkNow; i < connectionCount; i++) {
-            pool.add(oldPool.peek(i));
-        }
     }
 
     public int getIdleConnectionCount() {
-        return pool.currentSize();
+        return pool.size();
     }
 
 
     protected void getExpiredManagedConnectionInfos(long threshold, List<ManagedConnectionInfo>
killList) {
         synchronized (pool) {
-            for (int i = 0; i < pool.currentSize(); i++) {
-                ManagedConnectionInfo mci = pool.peek(i);
+            for (ManagedConnectionInfo mci : pool) {
                 if (mci.getLastUsed() < threshold) {
                     killList.add(mci);
                 }
             }
-        }
-    }
-
-    protected boolean addToPool(ManagedConnectionInfo mci) {
-        boolean added;
-        synchronized (pool) {
-            connectionCount++;
-            added = getPartitionMaxSize() > getIdleConnectionCount();
-            if (added) {
-                pool.add(mci);
-            }
-        }
-        return added;
-    }
-
-    static class PoolDeque {
-
-        private final ManagedConnectionInfo[] deque;
-        private final int first = 0;
-        private int last = -1;
-
-        public PoolDeque(int size) {
-            deque = new ManagedConnectionInfo[size];
-        }
-
-        //internal
-        public boolean isEmpty() {
-            return first > last;
-        }
-
-        //internal
-        public void add(ManagedConnectionInfo mci) {
-            if (last == deque.length - 1) {
-                throw new IllegalStateException("deque is full: contents: " + Arrays.asList(deque));
-            }
-            deque[++last] = mci;
-        }
-
-        //internal
-        public ManagedConnectionInfo peek(int i) {
-            if (i < first || i > last) {
-                throw new IllegalStateException("index is out of current range");
-            }
-            return deque[i];
-        }
-
-        //internal
-        public ManagedConnectionInfo removeLast() {
-            if (isEmpty()) {
-                throw new IllegalStateException("deque is empty");
-            }
-
-            return deque[last--];
-        }
-
-        //internal
-        public boolean remove(ManagedConnectionInfo mci) {
-            for (int i = first; i <= last; i++) {
-                if (deque[i] == mci) {
-                    for (int j = i + 1; j <= last; j++) {
-                        deque[j - 1] = deque[j];
-                    }
-                    last--;
-                    return true;
-                }
-
-            }
-            return false;
-        }
-
-        //internal
-        public int capacity() {
-            return deque.length;
-        }
-
-        //internal
-        public int currentSize() {
-            return last - first + 1;
         }
     }
 

Modified: geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllConnectionInterceptor.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllConnectionInterceptor.java?rev=620093&r1=620092&r2=620093&view=diff
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllConnectionInterceptor.java
(original)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/main/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllConnectionInterceptor.java
Sat Feb  9 01:16:29 2008
@@ -17,10 +17,11 @@
 
 package org.apache.geronimo.connector.outbound;
 
-import java.util.HashMap;
+import java.util.IdentityHashMap;
 import java.util.Iterator;
-import java.util.Map;
 import java.util.List;
+import java.util.Map;
+import java.util.ArrayList;
 
 import javax.resource.ResourceException;
 import javax.resource.spi.ManagedConnection;
@@ -37,9 +38,7 @@
  */
 public class SinglePoolMatchAllConnectionInterceptor extends AbstractSinglePoolConnectionInterceptor
{
 
-    private Map<ManagedConnection, ManagedConnectionInfo> pool;
-
-    private final int maxSize;
+    private final Map<ManagedConnection, ManagedConnectionInfo> pool;
 
     public SinglePoolMatchAllConnectionInterceptor(final ConnectionInterceptor next,
                                                    int maxSize,
@@ -48,8 +47,7 @@
                                                    int idleTimeoutMinutes) {
 
         super(next, maxSize, minSize, blockingTimeoutMilliseconds, idleTimeoutMinutes);
-        this.maxSize = maxSize;
-        pool = new HashMap<ManagedConnection, ManagedConnectionInfo>(maxSize);
+        pool = new IdentityHashMap<ManagedConnection, ManagedConnectionInfo>(maxSize);
     }
 
     protected void internalGetConnection(ConnectionInfo connectionInfo) throws ResourceException
{
@@ -57,88 +55,57 @@
             if (destroyed) {
                 throw new ResourceException("ManagedConnection pool has been destroyed");
             }
-            try {
-                if (!pool.isEmpty()) {
-                    ManagedConnectionInfo mci = connectionInfo.getManagedConnectionInfo();
-                    ManagedConnectionFactory managedConnectionFactory = mci.getManagedConnectionFactory();
-                    ManagedConnection matchedMC =
-                            managedConnectionFactory
-                            .matchManagedConnections(pool.keySet(),
-                                    mci.getSubject(),
-                                    mci.getConnectionRequestInfo());
-                    if (matchedMC != null) {
-                        connectionInfo.setManagedConnectionInfo(pool.get(matchedMC));
-                        pool.remove(matchedMC);
-                        if (log.isTraceEnabled()) {
-                            log.trace("Returning pooled connection " + connectionInfo.getManagedConnectionInfo());
-                        }
-                        if (connectionCount < minSize) {
-                            timer.schedule(new FillTask(connectionInfo), 10);
-                        }
-                        return;
+            if (!pool.isEmpty()) {
+                ManagedConnectionInfo mci = connectionInfo.getManagedConnectionInfo();
+                ManagedConnectionFactory managedConnectionFactory = mci.getManagedConnectionFactory();
+                ManagedConnection matchedMC =
+                        managedConnectionFactory
+                                .matchManagedConnections(pool.keySet(),
+                                        mci.getSubject(),
+                                        mci.getConnectionRequestInfo());
+                if (matchedMC != null) {
+                    connectionInfo.setManagedConnectionInfo(pool.get(matchedMC));
+                    pool.remove(matchedMC);
+                    if (log.isTraceEnabled()) {
+                        log.trace("Returning pooled connection " + connectionInfo.getManagedConnectionInfo());
                     }
+                    if (connectionCount < minSize) {
+                        timer.schedule(new FillTask(connectionInfo), 10);
+                    }
+                    return;
                 }
-                //matching failed or pool is empty
-                //if pool is at maximum size, pick a cx to kill
-                if (connectionCount == maxSize) {
-                    Iterator iterator = pool.entrySet().iterator();
-                    ManagedConnectionInfo kill = (ManagedConnectionInfo) ((Map.Entry) iterator.next()).getValue();
-                    iterator.remove();
-                    ConnectionInfo killInfo = new ConnectionInfo(kill);
-                    internalReturn(killInfo, ConnectionReturnAction.DESTROY);
-                }
-                next.getConnection(connectionInfo);
-                connectionCount++;
-                if (log.isTraceEnabled()) {
-                    log.trace("Returning new connection " + connectionInfo.getManagedConnectionInfo());
-                }
-                if (connectionCount < minSize) {
-                    timer.schedule(new FillTask(connectionInfo), 10);
-                }
-
-            } catch (ResourceException e) {
-                //something is wrong: rethrow, release permit
-                permits.release();
-                throw e;
             }
+            //matching failed or pool is empty
+            //if pool is at maximum size, pick a cx to kill
+            if (connectionCount == maxSize) {
+                Iterator iterator = pool.entrySet().iterator();
+                ManagedConnectionInfo kill = (ManagedConnectionInfo) ((Map.Entry) iterator.next()).getValue();
+                iterator.remove();
+                ConnectionInfo killInfo = new ConnectionInfo(kill);
+                internalReturn(killInfo, ConnectionReturnAction.DESTROY);
+            }
+            next.getConnection(connectionInfo);
+            connectionCount++;
+            if (log.isTraceEnabled()) {
+                log.trace("Returning new connection " + connectionInfo.getManagedConnectionInfo());
+            }
+            if (connectionCount < minSize) {
+                timer.schedule(new FillTask(connectionInfo), 10);
+            }
+
         }
     }
 
-    protected boolean internalReturn(ConnectionInfo connectionInfo, ConnectionReturnAction
connectionReturnAction) {
-        ManagedConnectionInfo mci = connectionInfo.getManagedConnectionInfo();
-        ManagedConnection mc = mci.getManagedConnection();
-        try {
-            mc.cleanup();
-        } catch (ResourceException e) {
-            connectionReturnAction = ConnectionReturnAction.DESTROY;
-        }
+    protected void doAdd(ManagedConnectionInfo mci) {
+        pool.put(mci.getManagedConnection(), mci);
+    }
 
-        boolean wasInPool = false;
-        synchronized (pool) {
-            // a bit redundant, but this closes a small timing hole...
-            if (destroyed) {
-                try {
-                    mc.destroy();
-                }
-                catch (ResourceException re) { } // ignore
-                return pool.remove(mci.getManagedConnection()) != null;
-            }
-            if (shrinkLater > 0) {
-                //nothing can get in the pool while shrinkLater > 0, so wasInPool is false
here.
-                connectionReturnAction = ConnectionReturnAction.DESTROY;
-                shrinkLater--;
-            } else if (connectionReturnAction == ConnectionReturnAction.RETURN_HANDLE) {
-                mci.setLastUsed(System.currentTimeMillis());
-                pool.put(mci.getManagedConnection(), mci);
-                return wasInPool;
-            } else {
-                wasInPool = pool.remove(mci.getManagedConnection()) != null;
-            }
-        }
-        //we must destroy connection.
-        next.returnConnection(connectionInfo, connectionReturnAction);
-        connectionCount--;
-        return wasInPool;
+    protected Object getPool() {
+        return pool;
+    }
+
+    protected boolean doRemove(ManagedConnectionInfo mci) {
+        return pool.remove(mci.getManagedConnection()) == null;
     }
 
     protected void internalDestroy() {
@@ -153,10 +120,6 @@
         }
     }
 
-    public int getPartitionMaxSize() {
-        return maxSize;
-    }
-
     public int getIdleConnectionCount() {
         synchronized (pool) {
             return pool.size();
@@ -164,23 +127,13 @@
     }
 
     protected void transferConnections(int maxSize, int shrinkNow) {
-        //1st example: copy 0 (none)
-        //2nd example: copy 10 (all)
-        synchronized (pool) {
-            Map<ManagedConnection, ManagedConnectionInfo> oldPool = pool;
-            Map<ManagedConnection, ManagedConnectionInfo> newPool = new HashMap<ManagedConnection,
ManagedConnectionInfo>(maxSize);
-            //since we have replaced pool already, pool.remove will be very fast:-)
-            assert oldPool.size() == connectionCount;
-            Iterator<Map.Entry<ManagedConnection, ManagedConnectionInfo>> it
= oldPool.entrySet().iterator();
-            for (int i = 0; i < shrinkNow; i++) {
-                ConnectionInfo killInfo = new ConnectionInfo((ManagedConnectionInfo) ((Map.Entry)it.next()).getValue());
-                internalReturn(killInfo, ConnectionReturnAction.DESTROY);
-            }
-            for (; it.hasNext(); ) {
-                Map.Entry<ManagedConnection, ManagedConnectionInfo> entry = it.next();
-                newPool.put(entry.getKey(), entry.getValue());
-            }
-            pool = newPool;
+        List<ConnectionInfo> killList = new ArrayList<ConnectionInfo>(shrinkNow);
+        Iterator<Map.Entry<ManagedConnection, ManagedConnectionInfo>> it = pool.entrySet().iterator();
+        for (int i = 0; i < shrinkNow; i++) {
+            killList.add(new ConnectionInfo(it.next().getValue()));
+        }
+        for (ConnectionInfo killInfo: killList) {
+            internalReturn(killInfo, ConnectionReturnAction.DESTROY);
         }
     }
 
@@ -193,18 +146,6 @@
             }
         }
 
-    }
-
-    protected boolean addToPool(ManagedConnectionInfo mci) {
-        boolean added;
-        synchronized (pool) {
-            connectionCount++;
-            added = getPartitionMaxSize() > getIdleConnectionCount();
-            if (added) {
-                pool.put(mci.getManagedConnection(), mci);
-            }
-        }
-        return added;
     }
 
 }

Added: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java?rev=620093&view=auto
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
(added)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
Sat Feb  9 01:16:29 2008
@@ -0,0 +1,272 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.apache.geronimo.connector.outbound;
+
+import java.util.List;
+import java.util.ArrayList;
+
+import javax.resource.ResourceException;
+import javax.resource.spi.ManagedConnectionFactory;
+
+import junit.framework.TestCase;
+import org.apache.geronimo.connector.mock.MockManagedConnectionFactory;
+
+/**
+ * @version $Rev:$ $Date:$
+ */
+public class AbstractSinglePoolTest extends TestCase {
+
+    private ManagedConnectionFactory mcf = new MockManagedConnectionFactory();
+    protected SwitchableInterceptor switchableInterceptor;
+    protected AbstractSinglePoolConnectionInterceptor interceptor;
+    protected int maxSize = 10;
+
+    protected void setUp() throws Exception {
+        ConnectionInterceptor interceptor = new MCFConnectionInterceptor();
+        switchableInterceptor = new SwitchableInterceptor(interceptor);
+    }
+
+    /**
+     * Check that you can only get maxSize connections out, whether you return or destroy
them.
+     * Check the pool state after each round.
+     * @throws Exception if test fails
+     */
+    public void testInitialMaxSize() throws Exception {
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        getConnections(ConnectionReturnAction.DESTROY);
+        assertEquals(0, interceptor.getConnectionCount());
+        assertEquals(0, interceptor.getIdleConnectionCount());
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+    }
+
+    /**
+     * Check that if connections cannot be created, no permits are expended.
+     * @throws Exception if test fails
+     */
+    public void testConnectionsUnavailable() throws Exception {
+        switchableInterceptor.setOn(false);
+        for (int i = 0; i < maxSize; i++) {
+            try {
+                getConnection();
+                fail("Connections should be unavailable");
+            } catch (ResourceException e) {
+                //pass
+            }
+        }
+        switchableInterceptor.setOn(true);
+        getConnections(ConnectionReturnAction.DESTROY);
+        switchableInterceptor.setOn(false);
+        for (int i = 0; i < maxSize; i++) {
+            try {
+                getConnection();
+                fail("Connections should be unavailable");
+            } catch (ResourceException e) {
+                //pass
+            }
+        }
+    }
+
+    /**
+     * Test that increasing the size of the pool has the expected effects on the connection
count
+     * and ability to get connections.
+     * @throws Exception if test fails
+     */
+    public void testResizeUp() throws Exception {
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        int maxSize = this.maxSize;
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        this.maxSize = 20;
+        interceptor.setPartitionMaxSize(this.maxSize);
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        assertEquals(this.maxSize, interceptor.getConnectionCount());
+        assertEquals(this.maxSize, interceptor.getIdleConnectionCount());
+    }
+
+    /**
+     * Check that decreasing the pool size while the pool is full (no connections checked
out)
+     * immediately destroys the expected number of excess connections and leaves the pool
full with
+     * the new max size
+     * @throws Exception if test fails
+     */
+    public void testResizeDown() throws Exception {
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        this.maxSize = 5;
+        interceptor.setPartitionMaxSize(this.maxSize);
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        assertEquals(this.maxSize, interceptor.getConnectionCount());
+        assertEquals(this.maxSize, interceptor.getIdleConnectionCount());
+    }
+
+    /**
+     * Check that, with all the connections checked out, shrinking the pool results in the
+     * expected number of connections being destroyed when they are returned.
+     * @throws Exception if test fails
+     */
+    public void testShrinkLater() throws Exception {
+        List<ConnectionInfo> cis = new ArrayList<ConnectionInfo>();
+        for (int i = 0; i < maxSize; i++) {
+            cis.add(getConnection());
+        }
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(0, interceptor.getIdleConnectionCount());
+        int oldMaxSize = maxSize;
+        maxSize = 5;
+        interceptor.setPartitionMaxSize(maxSize);
+        try {
+            getConnection();
+            fail("Pool should be exhausted");
+        } catch (ResourceException e) {
+            //pass
+        }
+        for (int i = 0; i< oldMaxSize - maxSize; i++) {
+            interceptor.returnConnection(cis.remove(0), ConnectionReturnAction.RETURN_HANDLE);
+        }
+        try {
+            getConnection();
+            fail("Pool should be exhausted");
+        } catch (ResourceException e) {
+            //pass
+        }
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(0, interceptor.getIdleConnectionCount());
+        for (int i = 0; i< maxSize; i++) {
+            interceptor.returnConnection(cis.remove(0), ConnectionReturnAction.RETURN_HANDLE);
+        }
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+    }
+
+    /**
+     * Check that the calculation of "shrinkLater" is correct when the pool is shrunk twice
before
+     * all the "shrinkLater" connections are returned. 
+     * @throws Exception if test fails
+     */
+    public void testShrinkLaterTwice() throws Exception {
+        List<ConnectionInfo> cis = new ArrayList<ConnectionInfo>();
+        for (int i = 0; i < maxSize; i++) {
+            cis.add(getConnection());
+        }
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(0, interceptor.getIdleConnectionCount());
+        int oldMaxSize = maxSize;
+        maxSize = 7;
+        interceptor.setPartitionMaxSize(maxSize);
+        try {
+            getConnection();
+            fail("Pool should be exhausted");
+        } catch (ResourceException e) {
+            //pass
+        }
+        interceptor.returnConnection(cis.remove(0), ConnectionReturnAction.RETURN_HANDLE);
+        oldMaxSize--;
+        maxSize = 5;
+        interceptor.setPartitionMaxSize(maxSize);
+        try {
+            getConnection();
+            fail("Pool should be exhausted");
+        } catch (ResourceException e) {
+            //pass
+        }
+        for (int i = 0; i< oldMaxSize - maxSize; i++) {
+            interceptor.returnConnection(cis.remove(0), ConnectionReturnAction.RETURN_HANDLE);
+        }
+        try {
+            getConnection();
+            fail("Pool should be exhausted");
+        } catch (ResourceException e) {
+            //pass
+        }
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(0, interceptor.getIdleConnectionCount());
+        for (int i = 0; i< maxSize; i++) {
+            interceptor.returnConnection(cis.remove(0), ConnectionReturnAction.RETURN_HANDLE);
+        }
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+        getConnections(ConnectionReturnAction.RETURN_HANDLE);
+        assertEquals(maxSize, interceptor.getConnectionCount());
+        assertEquals(maxSize, interceptor.getIdleConnectionCount());
+    }
+
+    private void getConnections(ConnectionReturnAction connectionReturnAction) throws ResourceException
{
+        List<ConnectionInfo> cis = new ArrayList<ConnectionInfo>();
+        for (int i = 0; i < maxSize; i++) {
+            cis.add(getConnection());
+        }
+        try {
+            getConnection();
+            fail("Pool should be exhausted");
+        } catch (ResourceException e) {
+            //pass
+        }
+        for (ConnectionInfo ci: cis) {
+            interceptor.returnConnection(ci, connectionReturnAction);
+        }
+    }
+
+    private ConnectionInfo getConnection() throws ResourceException {
+        ManagedConnectionInfo mci = new ManagedConnectionInfo(mcf, null);
+        ConnectionInfo ci = new ConnectionInfo(mci);
+        interceptor.getConnection(ci);
+        return ci;
+    }
+
+    private static class SwitchableInterceptor implements ConnectionInterceptor {
+        private final ConnectionInterceptor next;
+        private boolean on = true;
+
+        private SwitchableInterceptor(ConnectionInterceptor next) {
+            this.next = next;
+        }
+
+        public void setOn(boolean on) {
+            this.on = on;
+        }
+
+        public void getConnection(ConnectionInfo connectionInfo) throws ResourceException
{
+            if (on) {
+                next.getConnection(connectionInfo);
+            } else {
+                throw new ResourceException();
+            }
+        }
+
+        public void returnConnection(ConnectionInfo connectionInfo, ConnectionReturnAction
connectionReturnAction) {
+            next.returnConnection(connectionInfo, connectionReturnAction);
+        }
+
+        public void destroy() {
+            next.destroy();
+        }
+    }
+}

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/AbstractSinglePoolTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java?rev=620093&view=auto
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
(added)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
Sat Feb  9 01:16:29 2008
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.apache.geronimo.connector.outbound;
+
+/**
+ * @version $Rev:$ $Date:$
+ */
+public class SinglePoolMatchAllTest extends AbstractSinglePoolTest{
+
+    protected void setUp() throws Exception {
+        super.setUp();
+//        interceptor = new SinglePoolConnectionInterceptor(switchableInterceptor, maxSize,
0, 100, 1, true);
+        this.interceptor = new SinglePoolMatchAllConnectionInterceptor(switchableInterceptor,
maxSize, 0, 100, 1);
+    }
+
+}
\ No newline at end of file

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolMatchAllTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Added: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
URL: http://svn.apache.org/viewvc/geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java?rev=620093&view=auto
==============================================================================
--- geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
(added)
+++ geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
Sat Feb  9 01:16:29 2008
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.apache.geronimo.connector.outbound;
+
+/**
+ * @version $Rev:$ $Date:$
+ */
+public class SinglePoolTest extends AbstractSinglePoolTest{
+
+    protected void setUp() throws Exception {
+        super.setUp();
+        interceptor = new SinglePoolConnectionInterceptor(switchableInterceptor, maxSize,
0, 100, 1, true);
+//        this.interceptor = new SinglePoolMatchAllConnectionInterceptor(switchableInterceptor,
maxSize, 0, 100, 1);
+    }
+
+}

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
------------------------------------------------------------------------------
    svn:keywords = Date Revision

Propchange: geronimo/components/txmanager/trunk/geronimo-connector/src/test/java/org/apache/geronimo/connector/outbound/SinglePoolTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain



Mime
View raw message