From 627149920ca5bb2c43c22a419eeb50962973d9a7 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Fri, 14 Sep 2012 23:11:07 +0000
Subject: [PATCH] Fix OPENDJ-590: ConnectionPool may return already closed/disconnected connections

---
 opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/FixedConnectionPool.java    |   58 +++++++++++++++++++++++++++++++++-------------------------
 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionPoolTestCase.java |    2 +-
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/FixedConnectionPool.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/FixedConnectionPool.java
index 5f5acc8..1cde3f2 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/FixedConnectionPool.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/FixedConnectionPool.java
@@ -575,36 +575,44 @@
     @Override
     public FutureResult<Connection> getConnectionAsync(
             final ResultHandler<? super Connection> handler) {
-        QueueElement holder;
-        synchronized (queue) {
-            if (isClosed) {
-                throw new IllegalStateException("FixedConnectionPool is already closed");
+        // Loop while iterating through stale connections (see OPENDJ-590).
+        for (;;) {
+            final QueueElement holder;
+            synchronized (queue) {
+                if (isClosed) {
+                    throw new IllegalStateException("FixedConnectionPool is already closed");
+                }
+
+                if (queue.isEmpty() || queue.getFirst().isWaitingFuture()) {
+                    holder = new QueueElement(handler);
+                    queue.add(holder);
+                } else {
+                    holder = queue.removeFirst();
+                }
             }
 
-            if (queue.isEmpty() || queue.getFirst().isWaitingFuture()) {
-                holder = new QueueElement(handler);
-                queue.add(holder);
+            if (!holder.isWaitingFuture()) {
+                // There was a completed connection attempt.
+                final Connection connection = holder.getWaitingConnection();
+                if (connection.isValid()) {
+                    final PooledConnection pooledConnection = new PooledConnection(connection);
+                    if (handler != null) {
+                        handler.handleResult(pooledConnection);
+                    }
+                    return new CompletedFutureResult<Connection>(pooledConnection);
+                } else {
+                    // Close the stale connection and try again.
+                    connection.close();
+                }
             } else {
-                holder = queue.removeFirst();
+                // Grow the pool if needed.
+                final FutureResult<Connection> future = holder.getWaitingFuture();
+                if (!future.isDone() && currentPoolSize.tryAcquire()) {
+                    factory.getConnectionAsync(connectionResultHandler);
+                }
+                return future;
             }
         }
-
-        if (!holder.isWaitingFuture()) {
-            // There was a completed connection attempt.
-            final Connection connection = holder.getWaitingConnection();
-            final PooledConnection pooledConnection = new PooledConnection(connection);
-            if (handler != null) {
-                handler.handleResult(pooledConnection);
-            }
-            return new CompletedFutureResult<Connection>(pooledConnection);
-        } else {
-            // Grow the pool if needed.
-            final FutureResult<Connection> future = holder.getWaitingFuture();
-            if (!future.isDone() && currentPoolSize.tryAcquire()) {
-                factory.getConnectionAsync(connectionResultHandler);
-            }
-            return future;
-        }
     }
 
     /**
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionPoolTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionPoolTestCase.java
index 61ec078..97e4a38 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionPoolTestCase.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionPoolTestCase.java
@@ -273,7 +273,7 @@
      * @throws Exception
      *             If an unexpected error occurred.
      */
-    @Test(enabled = false)
+    @Test
     public void testSkipStaleConnectionsOnGet() throws Exception {
         // Setup.
         final Connection connection1 = mock(Connection.class);

--
Gitblit v1.10.0