From 5d952ff68e4f2f6dc534339bd62c9eb6bfcff8b8 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Fri, 8 Apr 2022 15:49:43 +0100
Subject: [PATCH] Don't return connections to the pool if they've thrown
 exceptions.

---
 .../briarproject/bramble/db/JdbcDatabase.java | 57 +++++++++++++------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java
index 94b0c13d8..0dfe41bd9 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java
@@ -70,6 +70,7 @@ import static java.sql.Types.BOOLEAN;
 import static java.sql.Types.INTEGER;
 import static java.sql.Types.VARCHAR;
 import static java.util.Arrays.asList;
+import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
@@ -572,6 +573,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 		try {
 			if (closed) throw new DbClosedException();
 			txn = connections.poll();
+			logConnectionCounts();
 		} finally {
 			connectionsLock.unlock();
 		}
@@ -588,6 +590,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 						throw new DbClosedException();
 					}
 					openConnections++;
+					logConnectionCounts();
 					connectionsChanged.signalAll();
 				} finally {
 					connectionsLock.unlock();
@@ -599,42 +602,57 @@ abstract class JdbcDatabase implements Database<Connection> {
 		return txn;
 	}
 
+	@GuardedBy("connectionsLock")
+	private void logConnectionCounts() {
+		if (LOG.isLoggable(FINE)) {
+			LOG.fine(openConnections + " connections open, "
+					+ connections.size() + " in pool");
+		}
+	}
+
 	@Override
 	public void abortTransaction(Connection txn) {
+		// The transaction may have been aborted due to an earlier exception,
+		// so close the connection rather than returning it to the pool
 		try {
 			txn.rollback();
-			connectionsLock.lock();
-			try {
-				connections.add(txn);
-				connectionsChanged.signalAll();
-			} finally {
-				connectionsLock.unlock();
-			}
 		} catch (SQLException e) {
-			// Try to close the connection
 			logException(LOG, WARNING, e);
-			tryToClose(txn, LOG, WARNING);
-			// Whatever happens, allow the database to close
-			connectionsLock.lock();
-			try {
-				openConnections--;
-				connectionsChanged.signalAll();
-			} finally {
-				connectionsLock.unlock();
-			}
+		}
+		closeConnection(txn);
+	}
+
+	private void closeConnection(Connection txn) {
+		tryToClose(txn, LOG, WARNING);
+		connectionsLock.lock();
+		try {
+			openConnections--;
+			logConnectionCounts();
+			connectionsChanged.signalAll();
+		} finally {
+			connectionsLock.unlock();
 		}
 	}
 
 	@Override
 	public void commitTransaction(Connection txn) throws DbException {
+		// If the transaction commits successfully then return the connection
+		// to the pool, otherwise close it
 		try {
 			txn.commit();
+			returnConnectionToPool(txn);
 		} catch (SQLException e) {
+			logException(LOG, WARNING, e);
+			closeConnection(txn);
 			throw new DbException(e);
 		}
+	}
+
+	private void returnConnectionToPool(Connection txn) {
 		connectionsLock.lock();
 		try {
 			connections.add(txn);
+			logConnectionCounts();
 			connectionsChanged.signalAll();
 		} finally {
 			connectionsLock.unlock();
@@ -650,6 +668,10 @@ abstract class JdbcDatabase implements Database<Connection> {
 			openConnections -= connections.size();
 			connections.clear();
 			while (openConnections > 0) {
+				if (LOG.isLoggable(INFO)) {
+					LOG.info("Waiting for " + openConnections
+							+ " connections to be closed");
+				}
 				try {
 					connectionsChanged.await();
 				} catch (InterruptedException e) {
@@ -660,6 +682,7 @@ abstract class JdbcDatabase implements Database<Connection> {
 				openConnections -= connections.size();
 				connections.clear();
 			}
+			LOG.info("All connections closed");
 		} finally {
 			connectionsLock.unlock();
 		}