When connection fails, partition may be decremented before the connection is added to it

[James Roper]

New connections are added to a partition by invoking the ConnectionHandle constructor, and then passing the ConnectionHandle to addFreeConnection (BoneCP.java:466, PoolWatchThread.java:115). So, the connection will always be added to the partition after it's created, and won't be added if it fails to be created.

If creating the connection fails, markPossiblyBroken is invoked from the ConnectionHandle constructor (ConnectionHandle.java:243). The problem with invoking this in the constructor is that markPossiblyBroken assumes that the connection is already in the partition, which is not true if it's a new connection, and does, among other things, decrement the partition connection count (through pool.destroyConnection). This means that when the database goes down, and BoneCP attempts to reconnect many times, the partition connection count ends up being negative, and when the database comes back up, many more connections are created than should be allowed.

As for a solution, it seems that markPossiblyBroken should not be invoked if the connection is a new connection. Much of what it does seems to me to not make sense (anything it does on itself is irrelevant because the constructor never returns, the clean up it does on the pool is dangerous because the connection was never in the pool in the first place, and I'm not sure if it makes sense to invoke the connection hook at this point or not). But I'm not familiar enough with bonecp to know if that's right or not, the main thing I'm not sure about is whether all connections should be terminated and the dbIsDown flag should be set - if that is the case, perhaps that logic should be pulled out into a separate method, and the constructor should invoke that for failed new connections.