[Opensource] DBConnectionPool Infinite Loop

[Javier Campoamor]

I have seen a dead lock in DBConnection. The problem is that when I
configure a context to use a test query when a conection is requested,
if there is more than one connection and they are broken, the
DBConnectionPool is unable asign a connection.

In line 747 we cannot get a connection because the connection is broken
(I've restarted the database):

 		DBConnection oneConnection = findExistingConnection();


Later in line 775 we go into a loop to get another connection:

while (!finished) {
	boolean createNew = false;

	if (inUse.size() >= maxPoolSize) {
		if (log.isInfoEnabled()) {
			log.info("Pool '" + getDBName() + "' is full -
trying clean");
		}

		/* run the cleanup, in case we have an old, stale
connection */
		clean();
		oneConnection = findExistingConnection();
		if (oneConnection != null) {
			if (log.isDebugEnabled()) {
				log.debug("Found existing connection
after clean");
			}
			return oneConnection;
		}
	}

	/* now see if we have room */
	if ((inUse.size() + available.size()) >= maxPoolSize) {
		log.warn("--------------------------------");
		log.warn("WARNING: DB Connection Pool '" + getDBName() +
				"' is " + "full even after clean.");
		//
		//Extensive Debugging Output if needed
		//
		if (log.isDebugEnabled()) {
			synchronized (poolLock) {
				int i = 0;
				log.debug("Current contents:");
				for (Iterator conns =
inUse.values().iterator();
					 conns.hasNext();) {
					DBConnection oneConn =
(DBConnection) conns.next();
					i++;
					Date dt = new
Date(oneConn.getLastTouched());
					Calendar c =
Calendar.getInstance();
					c.setTime(dt);
					log.debug("Connection " + i +
":" +
	
oneConn.getDescription() + ":" +
	
DateTime.getDateString(c));
				}
				log.debug("Waiting " + (interval / 2000)
+
						" seconds for a
connection to free up");
			}
		}

		/* Now wait for a number of seconds, retrying again */
		long currentTime = System.currentTimeMillis();
		long finaltime = currentTime + (interval / 2);
		try {
			while (finaltime > currentTime) {
				synchronized (poolLock) {
					poolLock.wait(finaltime -
System.currentTimeMillis());
				}
				//
				//a piece became available... let's see
if we can get it
				//
				clean();
				oneConnection =
findExistingConnection();
				if (oneConnection != null) {
					if (log.isDebugEnabled()) {
						log.debug("Found
existing connection after sleep and clean");
					}

	
oneConnection.setDBName(getDBName());
					return oneConnection;
				}
				currentTime =
System.currentTimeMillis();
			}

			//
			//Throw a PoolFull Exception because we timed
out waiting
			//for a connection
			//
			if (inUse.size() >= maxPoolSize) {
				log.error("Pool '" + getDBName() + "'
still full");
				throw new PoolFullException("Cannot
allocate " +
						"another database
connection. There are already " +
						inUse.size() +
						" connections in use." +
						" Please report this
problem to the System " +
						"Administrator");
			}

		} catch (InterruptedException ie) {
			throw new ConnectionPoolException("Interrupted
while waiting for available connection");
		}
	} else {
		createNew = false;
		//
		//We need an accurate count here, thus the lock
		//
		synchronized (poolLock) {
			if (totalPools == 0) {
				firstConnection = true;
			}

			if (totalPools < maxPoolSize) {
				createNew = true;
			}
		}

/*
As we don't have more connections than the maximum, we reach this point,
but when we try to create a new connection,
we get a null connection because already exists connections, and the
"while(!finished)" loop never ends.
*/
		if (createNew) {
			oneConnection =
createNewConnection(firstConnection);
			if (oneConnection != null) {
				return oneConnection;
			}
		}
	}
}






I think that the solution is to modify findExistingConnection to
continue itetating over the available connections till there is no more
connections. Whit this change, the findExistingConnection returns null
only if there is no more available connections, and the
createNewConnection can return a new one (I don't know if
createNewConnection should return a new connection allways.)



		/* Check for a dead connection */
		if (testQuery != null) {
			try {
				System.out.println("totalPools = " +
totalPools);
				oneConnection.execute(testQuery);
			} catch (DBException de) {
				if (log.isDebugEnabled()) {
					log.debug("Test query '" +
testQuery +
							"' failed:" +
de.getMessage() +
							", closing
connection & trying again.\ntotalPools = " + totalPools);
				}
				try {
					oneConnection.disconnect();
				} catch (Exception e) {
					log.error("Unable to close
connection that failed" +
							"test query",
e);
				}


<jcampoamor>
				// We have to decrease the number of
total pools 
				// (otherwise we are counting
connections that no longer exists)
				totalPools --;
				// This was the cause of a bug because
it returns a null while we could have
				// more available connections. What we
have to do is to continue iterating
				// and decrease the number of total
pools.
//				return null;
				continue;
</jcampoamor>

			} // catch (DBException de)
		} // if (testQuery != null)



Best regards,

Javier