Alright, there is actually an infinite loop in DbConnectionBroker.
Check it out. All of this code is in
DbConnectionBroker.getConnection():
do {
synchronized (this) {
ConnDescription desc =
(ConnDescription) pool.elementAt(roundRobin);
if (desc.getStatus() == AVAILABLE) {
// Connection is available
* if (!desc.getConnection().isClosed()) {
conn = desc.getConnection();
SQLWarning currSQLWarning =
desc.getConnection().getWarnings();
if (currSQLWarning != null) {
log.println("Warnings on connection "
+ desc.getId() + " "
+ currSQLWarning);
desc.getConnection().clearWarnings();
}
desc.setStatus(ACTIVE);
desc.setLockTime(System.currentTimeMillis());
desc.setTrace(new Throwable());
connLast = roundRobin;
return conn;
}
} else {
loop++;
if (++roundRobin >= pool.size()) {
roundRobin = 0;
}
}
} // synchronized
} while (loop < pool.size());
See the if statement marked by the *? Well, there's no else, so if
the connection is closed it skips right down to the } while (loop...
line at the bottom and continues to loop. loop never gets
incremented when this happens, so it's an infinite loop. I've
patched this in my own code, but I haven't committed it to the
repository yet. Why? Well... There's a few things I want to clear
up before I consider the DbConnectionBroker code "fixed".
Alright, so if a connection isClosed(), it still stays in the pool
for the duration of this loop. I guess that's fine, but when does
this ConnDescription get a new unclosed connection, or should the
ConnDescription just be replaced by a new one?
I looked through the run() method and there does not appear to be any
code in it that checks for and removes closed connections, only ones
that are 'too old'.
Other question. See the code below. It follows right where the code
above ends:
} catch (SQLException ex) {
// Ignore any exceptions
}
synchronized (this) {
if (pool.size() < maxConns) {
// Let's create another connection
try {
ConnDescription desc = createConn(log);
SQLWarning currSQLWarning =
desc.getConnection().getWarnings();
if (currSQLWarning != null) {
log.println("Warnings on connection "
+ desc.getId() + " "
+ currSQLWarning);
desc.getConnection().clearWarnings();
}
desc.setTrace(new Throwable());
return desc.getConnection();
} catch (SQLException e) {
log.println(new Date()
+ " Unable to create new connection: "
+ e);
}
}
}
Alright, so if all the connections in the pool are exhausted it
creates a new ConnDescription, which apparently also creates a new
connection. But... does this new connection ever get added to the
pool? Doesn't look like it to me.
Would anyone object if I just rewrote getConnection() from scratch?
Or would someone else like to do that? Would the person who
originally wrote this code please let me know if I'm assuming
anything wrong about how this system is supposed to work so that
if/when I re-write it, I don't do it differently than it was intended?
Thanks,
Avi Cherry
------------------------------------------------------------
To subscribe: [EMAIL PROTECTED]
To unsubscribe: [EMAIL PROTECTED]
Problems?: [EMAIL PROTECTED]
