diff --git a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/pool/ConnectionPool.java b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/pool/ConnectionPool.java
index 390e1144090..6a99a0f12cb 100644
--- a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/pool/ConnectionPool.java
+++ b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/pool/ConnectionPool.java
@@ -44,6 +44,7 @@
import java.util.List;
import java.util.Set;
import java.util.Timer;
+import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Logger;
import javax.naming.NamingException;
@@ -67,20 +68,80 @@ public class ConnectionPool implements ResourcePool, ConnectionLeakListener, Res
private static final Logger LOG = LogDomains.getLogger(ConnectionPool.class, LogDomains.RSR_LOGGER);
// pool life-cycle config properties
+ /**
+ * Represents the "max-pool-size" configuration value.
+ * Specifies the maximum number of connections that can be created to satisfy client requests.
+ * Default: 32
+ */
protected int maxPoolSize;
+
+ /**
+ * Represents the "steady-pool-size" configuration value.
+ * Specifies the initial and minimum number of connections maintained in the pool.
+ * Default: 8
+ */
protected int steadyPoolSize;
- /** used by resizer to downsize the pool */
+
+ /**
+ * Represents the "pool-resize-quantity" configuration value.
+ * Specifies the number of idle connections to be destroyed if the existing number of connections is above the
+ * steady-pool-size (subject to the max-pool-size limit).
+ * This is enforced periodically at the idle-timeout-in-seconds interval. An idle connection is one that has not been
+ * used for a period of idle-timeout-in-seconds. When the pool size reaches steady-pool-size, connection removal
+ * stops.
+ * Default: 2
+ */
protected int resizeQuantity;
- /** The total time a thread is willing to wait for a resource object. */
+
+ /**
+ * Represents the "max-wait-time-in-millis" configuration value.
+ * Specifies the amount of time, in milliseconds, that the caller is willing to wait for a connection. If 0, the caller
+ * is blocked indefinitely until a resource is available or an error occurs.
+ * Default: 60.000ms
+ */
protected int maxWaitTime;
- /** time (in ms) before destroying a free resource */
+
+ /**
+ * Represents the "idle-timeout-in-seconds" configuration value, but in millis.
+ * Specifies the maximum time that a connection can remain idle in the pool. After this amount of time, the pool can
+ * close this connection.
+ * Default: 300.000ms
+ */
protected long idletime;
// pool config properties
+ /**
+ * Represents the "fail-all-connections" configuration value.
+ * If true, closes all connections in the pool if a single validation check fails.
+ * Default: false
+ */
protected boolean failAllConnections;
+
+ /**
+ * Represents the "match-connections" configuration value.
+ * If true, enables connection matching. You can set to false if connections are homogeneous.
+ * Default: true
+ */
protected boolean matchConnections;
+ /**
+ * Represents the "is-connection-validation-required" configuration value.
+ * Specifies whether connections have to be validated before being given to the application. If a resource’s validation
+ * fails, it is destroyed, and a new resource is created and returned.
+ * Default: false
+ *
+ * TODO: rename to 'connectionValidationRequired'
+ */
protected boolean validation;
+
+ /**
+ * Represents the "prefer-validate-over-recreate property" configuration value.
+ * Specifies that validating idle connections is preferable to closing them. This property has no effect on non-idle
+ * connections. If set to true, idle connections are validated during pool resizing, and only those found to be invalid
+ * are destroyed and recreated. If false, all idle connections are destroyed and recreated during pool resizing.
+ * Default: false
+ */
protected boolean preferValidateOverRecreate;
+
// hold on to the resizer task so we can cancel/reschedule it.
protected Resizer resizerTask;
@@ -88,17 +149,55 @@ public class ConnectionPool implements ResourcePool, ConnectionLeakListener, Res
protected Timer timer;
// advanced pool config properties
+ /**
+ * Represents the "connection-creation-retry-attempts" configuration value.
+ * If the value connectionCreationRetryAttempts_ is > 0 then connectionCreationRetry_ is true, otherwise false.
+ */
protected boolean connectionCreationRetry_;
+ /**
+ * Represents the "connection-creation-retry-attempts" configuration.
+ * Specifies the number of attempts to create a new connection.
+ * Default: 0
+ */
protected int connectionCreationRetryAttempts_;
+
+ /**
+ * Represents the "connection-creation-retry-interval-in-seconds" configuration, but in millis.
+ * Specifies the time interval between attempts to create a connection when connection-creation-retry-attempts is
+ * greater than 0.
+ * Default: 10.000ms
+ */
protected long conCreationRetryInterval_;
+
+ /**
+ * Represents the "validate-atmost-once-period-in-seconds" configuration, but in millis.
+ * Specifies the time interval within which a connection is validated at most once. Minimizes the number of validation
+ * calls. A value of zero allows unlimited validation calls.
+ * Default: 10.000ms.
+ */
protected long validateAtmostPeriodInMilliSeconds_;
+
+ /**
+ * Represents the "max-connection-usage-count" configuration.
+ * Specifies the number of times a connections is reused by the pool, after which it is closed. A zero value disables
+ * this feature.
+ * Default: 0
+ */
protected int maxConnectionUsage_;
- // To validate a Sun RA Pool Connection if it hasnot been validated
+
+ // To validate a Sun RA Pool Connection if it has not been validated
// in the past x sec. (x=idle-timeout)
// The property will be set from system property -
// com.sun.enterprise.connectors.ValidateAtmostEveryIdleSecs=true
+ /**
+ * Represents the "validate-atmost-once-period-in-seconds" configuration.
+ * Specifies the time interval within which a connection is validated at most once. Minimizes the number of validation
+ * calls. A value of zero allows unlimited validation calls.
+ * Default: 0
+ */
private boolean validateAtmostEveryIdleSecs;
+ // TODO document
protected String resourceSelectionStrategyClass;
protected PoolLifeCycleListener poolLifeCycleListener;
@@ -129,6 +228,13 @@ public class ConnectionPool implements ResourcePool, ConnectionLeakListener, Res
private boolean blocked;
+ /**
+ * Reentrant lock to ensure calls to {@link #getResourceFromPool(ResourceAllocator, ResourceSpec)} and
+ * {@link #freeResource(ResourceHandle)} are not executed at the same time to solve issue 24843, because one is getting
+ * resources from the pool and possibly resizing the pool while the other is returning resources to the pool.
+ */
+ private final ReentrantLock getResourceFromPoolAndFreeResourceMethodsLock = new ReentrantLock(true);
+
public ConnectionPool(PoolInfo poolInfo, Hashtable env) throws PoolingException {
this.poolInfo = poolInfo;
setPoolConfiguration(env);
@@ -675,50 +781,55 @@ protected ResourceHandle getResourceFromPool(ResourceAllocator resourceAllocator
ResourceHandle resourceHandle;
List freeResources = new ArrayList<>();
- try {
- while ((resourceHandle = dataStructure.getResource()) != null) {
- if (resourceHandle.hasConnectionErrorOccurred()) {
- dataStructure.removeResource(resourceHandle);
- continue;
- }
+ try {
+ getResourceFromPoolAndFreeResourceMethodsLock.lock();
+ try {
+ while ((resourceHandle = dataStructure.getResource()) != null) {
+ if (resourceHandle.hasConnectionErrorOccurred()) {
+ dataStructure.removeResource(resourceHandle);
+ continue;
+ }
- if (matchConnection(resourceHandle, resourceAllocator)) {
+ if (matchConnection(resourceHandle, resourceAllocator)) {
- boolean isValid = isConnectionValid(resourceHandle, resourceAllocator);
- if (resourceHandle.hasConnectionErrorOccurred() || !isValid) {
- if (failAllConnections) {
- resourceFromPool = createSingleResourceAndAdjustPool(resourceAllocator, resourceSpec);
- // No need to match since the resource is created with the allocator of caller.
+ boolean isValid = isConnectionValid(resourceHandle, resourceAllocator);
+ if (resourceHandle.hasConnectionErrorOccurred() || !isValid) {
+ if (failAllConnections) {
+ resourceFromPool = createSingleResourceAndAdjustPool(resourceAllocator, resourceSpec);
+ // No need to match since the resource is created with the allocator of caller.
+ break;
+ }
+ dataStructure.removeResource(resourceHandle);
+ // Resource is invalid, continue iteration.
+ continue;
+ }
+ if (resourceHandle.isShareable() == resourceAllocator.shareableWithinComponent()) {
+ // Got a matched, valid resource
+ resourceFromPool = resourceHandle;
break;
}
- dataStructure.removeResource(resourceHandle);
- // Resource is invalid, continue iteration.
- continue;
- }
- if (resourceHandle.isShareable() == resourceAllocator.shareableWithinComponent()) {
- // Got a matched, valid resource
- resourceFromPool = resourceHandle;
- break;
+ freeResources.add(resourceHandle);
+ } else {
+ freeResources.add(resourceHandle);
}
- freeResources.add(resourceHandle);
- } else {
- freeResources.add(resourceHandle);
}
+ } finally {
+ // Return all unmatched, free resources
+ for (ResourceHandle freeResource : freeResources) {
+ dataStructure.returnResource(freeResource);
+ }
+ freeResources.clear();
}
- } finally {
- // Return all unmatched, free resources
- for (ResourceHandle freeResource : freeResources) {
- dataStructure.returnResource(freeResource);
- }
- freeResources.clear();
- }
- if (resourceFromPool != null) {
- // Set correct state
- setResourceStateToBusy(resourceFromPool);
- } else {
- resourceFromPool = resizePoolAndGetNewResource(resourceAllocator);
+ if (resourceFromPool != null) {
+ // Set correct state
+ setResourceStateToBusy(resourceFromPool);
+ } else {
+ resourceFromPool = resizePoolAndGetNewResource(resourceAllocator);
+ }
+ } finally {
+ getResourceFromPoolAndFreeResourceMethodsLock.unlock();
}
return resourceFromPool;
@@ -734,26 +845,27 @@ protected ResourceHandle getResourceFromPool(ResourceAllocator resourceAllocator
* @throws PoolingException when not able to create resources
*/
private ResourceHandle resizePoolAndGetNewResource(ResourceAllocator resourceAllocator) throws PoolingException {
+
// Must be called from the thread holding the lock to this pool.
ResourceHandle newResource = null;
int numOfConnsToCreate = 0;
- if (dataStructure.getResourcesSize() < steadyPoolSize) {
+ int dataStructureSize = dataStructure.getResourcesSize();
+ if (dataStructureSize < steadyPoolSize) {
// May be all invalid resources are destroyed as
// a result no free resource found and no. of resources is less than steady-pool-size
- numOfConnsToCreate = steadyPoolSize - dataStructure.getResourcesSize();
- } else if (dataStructure.getResourcesSize() + resizeQuantity <= maxPoolSize) {
-
+ numOfConnsToCreate = steadyPoolSize - dataStructureSize;
+ } else if (dataStructureSize + resizeQuantity <= maxPoolSize) {
// Create and add resources of quantity "resizeQuantity"
numOfConnsToCreate = resizeQuantity;
- } else if (dataStructure.getResourcesSize() < maxPoolSize) {
-
+ } else if (dataStructureSize < maxPoolSize) {
// This else if "test condition" is not needed. Just to be safe.
// still few more connections (less than "resizeQuantity" and to reach the count of maxPoolSize)
// can be added
- numOfConnsToCreate = maxPoolSize - dataStructure.getResourcesSize();
+ numOfConnsToCreate = maxPoolSize - dataStructureSize;
}
+
if (numOfConnsToCreate > 0) {
createResources(resourceAllocator, numOfConnsToCreate);
newResource = getMatchedResourceFromPool(resourceAllocator);
@@ -962,7 +1074,9 @@ public void resourceClosed(ResourceHandle handle) throws IllegalStateException {
if (poolLifeCycleListener != null && !handle.getDestroyByLeakTimeOut()) {
poolLifeCycleListener.connectionReleased(handle.getId());
}
- LOG.log(FINE, "Resource was freed after it`s closure: {0}", handle);
+
+ // Note handle might already be altered by another thread before it is logged!
+ LOG.log(FINE, "Resource was freed after its closure: {0}", handle);
}
/**
@@ -994,23 +1108,29 @@ protected void freeUnenlistedResource(ResourceHandle h) {
}
protected void freeResource(ResourceHandle resourceHandle) {
- if (cleanupResource(resourceHandle)) {
- // Only when resource handle usage count is more than maxConnUsage
- if (maxConnectionUsage_ > 0 && resourceHandle.getUsageCount() >= maxConnectionUsage_) {
- performMaxConnectionUsageOperation(resourceHandle);
- } else {
- // Put it back to the free collection.
- dataStructure.returnResource(resourceHandle);
- // update the monitoring data
- if (poolLifeCycleListener != null && !resourceHandle.getDestroyByLeakTimeOut()) {
- poolLifeCycleListener.decrementConnectionUsed(resourceHandle.getId());
- poolLifeCycleListener.incrementNumConnFree(false, steadyPoolSize);
+ try {
+ getResourceFromPoolAndFreeResourceMethodsLock.lock();
+ if (cleanupResource(resourceHandle)) {
+ // Only when resource handle usage count is more than maxConnUsage
+ if (maxConnectionUsage_ > 0 && resourceHandle.getUsageCount() >= maxConnectionUsage_) {
+ performMaxConnectionUsageOperation(resourceHandle);
+ } else {
+ // Put it back to the free collection.
+ dataStructure.returnResource(resourceHandle);
+ // update the monitoring data
+ if (poolLifeCycleListener != null && !resourceHandle.getDestroyByLeakTimeOut()) {
+ poolLifeCycleListener.decrementConnectionUsed(resourceHandle.getId());
+ poolLifeCycleListener.incrementNumConnFree(false, steadyPoolSize);
+ }
}
+ // For both the cases of free.add and maxConUsageOperation, a free resource is added.
+ // Hence notify waiting threads.
+ notifyWaitingThreads();
}
- // for both the cases of free.add and maxConUsageOperation, a free resource is added.
- // Hence notify waiting threads
- notifyWaitingThreads();
+ } finally {
+ getResourceFromPoolAndFreeResourceMethodsLock.unlock();
}
+
}
protected boolean cleanupResource(ResourceHandle handle) {
@@ -1066,6 +1186,10 @@ public void resourceErrorOccurred(ResourceHandle resourceHandle) throws IllegalS
// removed in the next iteration of getUnenlistedResource
// or internalGetResource
dataStructure.removeResource(resourceHandle);
+
+ // Removing a resource, means a free resource is available for the pool.
+ // Hence notify waiting threads.
+ notifyWaitingThreads();
}
private void doFailAllConnectionsProcessing() {
diff --git a/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/pool/ConnectionPoolTest.java b/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/pool/ConnectionPoolTest.java
index 1b7daba9e1f..e45c5d96214 100644
--- a/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/pool/ConnectionPoolTest.java
+++ b/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/pool/ConnectionPoolTest.java
@@ -28,15 +28,22 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
import java.util.Hashtable;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import org.easymock.IExpectationSetters;
import org.glassfish.api.admin.ProcessEnvironment;
@@ -46,7 +53,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
-import org.opentest4j.MultipleFailuresError;
import com.sun.appserv.connectors.internal.api.ConnectorRuntimeException;
import com.sun.appserv.connectors.internal.api.PoolingException;
@@ -59,16 +65,23 @@
import com.sun.enterprise.resource.ResourceState;
import com.sun.enterprise.resource.allocator.LocalTxConnectorAllocator;
import com.sun.enterprise.resource.allocator.ResourceAllocator;
-import com.sun.enterprise.resource.pool.datastructure.RWLockDataStructure;
+import com.sun.enterprise.resource.pool.datastructure.DataStructure;
import com.sun.enterprise.transaction.api.JavaEETransaction;
+import com.sun.logging.LogDomains;
import jakarta.resource.ResourceException;
import jakarta.resource.spi.ManagedConnection;
import jakarta.resource.spi.ManagedConnectionFactory;
+import jakarta.resource.spi.RetryableUnavailableException;
import jakarta.transaction.Transaction;
public class ConnectionPoolTest {
+ private static final Logger LOG = LogDomains.getLogger(ConnectionPool.class, LogDomains.RSR_LOGGER);
+
+ private static final String NO_AVAILABLE_RESOURCES_AND_WAIT_TIME = "No available resources and wait time ";
+ private static final String MS_EXPIRED = " ms expired.";
+
private ManagedConnection managedConnection;
private ManagedConnectionFactory managedConnectionFactory;
private JavaEETransaction javaEETransaction;
@@ -77,6 +90,9 @@ public class ConnectionPoolTest {
private ResourceSpec resourceSpec;
+ // TODO: Look at tests like: org.glassfish.jdbc.devtests.v3.test.MaxConnectionUsageTest and the others in the package,
+ // they can probably also be made as a unit test here / or in a similar unit test
+
@BeforeEach
public void createAndPopulateMocks() throws PoolingException, ResourceException {
List