From f0eb67cff05c0fc3349eb8642e15ecb7be445737 Mon Sep 17 00:00:00 2001 From: Fernanda Meheust Date: Mon, 6 Nov 2023 15:28:08 +0100 Subject: [PATCH 1/3] Added calls to beginRequest and endRequest --- modules/jdbc-pool/build.properties.default | 4 +- .../tomcat/jdbc/pool/ConnectionPool.java | 47 ++++++++++++++++ .../jdbc/test/ConnectionBoundariesTest.java | 53 +++++++++++++++++++ .../tomcat/jdbc/test/driver/Connection.java | 6 +++ .../tomcat/jdbc/test/driver/Driver.java | 3 ++ 5 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java diff --git a/modules/jdbc-pool/build.properties.default b/modules/jdbc-pool/build.properties.default index e300ecb79762..78a7b868482c 100644 --- a/modules/jdbc-pool/build.properties.default +++ b/modules/jdbc-pool/build.properties.default @@ -91,10 +91,10 @@ tomcat.dbcp.jar=${tomcat.home}/lib/tomcat-dbcp.jar tomcat.juli.jar=${tomcat.home}/bin/tomcat-juli.jar tomcat.loc=https://archive.apache.org/dist/tomcat/tomcat-8/v${tomcat.version}/bin/apache-tomcat-${tomcat.version}.zip -tomcat.project.loc=https://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/project.xml +tomcat.project.loc=https://github.com/apache/tomcat/tree/main/webapps/docs/project.xml tomcat.project.dest=${base.path}/project.xml -tomcat.xsl.loc=https://svn.apache.org/repos/asf/tomcat/trunk/webapps/docs/tomcat-docs.xsl +tomcat.xsl.loc=https://github.com/apache/tomcat/tree/main/webapps/docs/tomcat-docs.xsl tomcat.xsl.dest=${base.path}/tomcat-docs.xsl derby.home=${base.path}/db-derby-10.5.1.1-bin diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index 2d02f2488468..24f4c479ff75 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -19,7 +19,9 @@ import java.lang.ref.WeakReference; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Proxy; +import java.lang.reflect.Method; import java.sql.Connection; import java.sql.SQLException; import java.util.Collections; @@ -138,6 +140,13 @@ public class ConnectionPool { private final AtomicLong removeAbandonedCount = new AtomicLong(0); private final AtomicLong releasedIdleCount = new AtomicLong(0); + /** + * Request boundaries + */ + private volatile boolean requestBoundaryMethodsInitialised = false; + private Method beginRequest = null; + private Method endRequest = null; + //=============================================================================== // PUBLIC METHODS //=============================================================================== @@ -343,6 +352,14 @@ protected Connection setupConnection(PooledConnection con) throws SQLException { } else { connection = (Connection)proxyClassConstructor.newInstance(new Object[] {handler}); } + Connection underlyingConnection = con.getConnection(); + if (hasRequestBoundaryMethods(underlyingConnection)) { + try { + beginRequest.invoke(underlyingConnection); + } catch (InvocationTargetException | IllegalAccessException ex) { + log.warn("Error calling beginRequest on connection", ex); + } + } //return the connection return connection; }catch (Exception x) { @@ -812,6 +829,28 @@ protected PooledConnection createConnection(long now, PooledConnection notUsed, }//catch } + /** + * If request boundaries are not initialised, checks if beginRequest and + * endRequest methods are implemented on connection object. + * + * @param connection The connection + * @return Returns true if connection is not null and connection implements + * JDBC 4.3 beginRequest and endRequest methods + */ + private boolean hasRequestBoundaryMethods(Connection connection) { + if (!requestBoundaryMethodsInitialised && connection != null) { + try { + beginRequest = connection.getClass().getMethod("beginRequest"); + endRequest = connection.getClass().getMethod("endRequest"); + } catch (NoSuchMethodException ex) { + // begin and end request not implemented, will not be invoked + } finally { + requestBoundaryMethodsInitialised = true; + } + } + return (connection != null && beginRequest != null && endRequest != null); + } + /** * Validates and configures a previously idle connection * @param now - timestamp @@ -1027,6 +1066,14 @@ protected void returnConnection(PooledConnection con) { con.clearWarnings(); con.setStackTrace(null); con.setTimestamp(System.currentTimeMillis()); + Connection underlyingConnection = con.getConnection(); + if (hasRequestBoundaryMethods(underlyingConnection)) { + try { + endRequest.invoke(underlyingConnection); + } catch (InvocationTargetException | IllegalAccessException ex) { + log.warn("Error calling endRequest on connection", ex); + } + } if (((idle.size()>=poolProperties.getMaxIdle()) && !poolProperties.isPoolSweeperEnabled()) || (!idle.offer(con))) { if (log.isDebugEnabled()) { log.debug("Connection ["+con+"] will be closed and not returned to the pool, idle["+idle.size()+"]>=maxIdle["+poolProperties.getMaxIdle()+"] idle.offer failed."); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java new file mode 100644 index 000000000000..e9b2e98e23b2 --- /dev/null +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.jdbc.test; + +import org.apache.tomcat.jdbc.test.driver.Driver; + +import java.sql.Connection; + +import org.junit.Test; +import org.junit.Assert; + +public class ConnectionBoundariesTest extends DefaultTestCase { + + + @Override + public org.apache.tomcat.jdbc.pool.DataSource createDefaultDataSource() { + // TODO Auto-generated method stub + org.apache.tomcat.jdbc.pool.DataSource ds = super.createDefaultDataSource(); + ds.getPoolProperties().setDriverClassName(Driver.class.getName()); + ds.getPoolProperties().setUrl(Driver.url); + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setMaxIdle(10); + ds.getPoolProperties().setMinIdle(10); + ds.getPoolProperties().setMaxActive(10); + return ds; + } + @Test + public void connectionWithRequestBoundariesTest() throws Exception { + datasource.getPoolProperties().getDbProperties().setProperty("requestBoundaries", "true"); + Connection connection = datasource.getConnection(); + Assert.assertEquals("Connection.beginRequest() count", 1, Driver.beginRequestCount.get()); + Assert.assertEquals("Connection.endRequest() count", 0, Driver.endRequestCount.get()); + connection.close(); + Assert.assertEquals("Connection.beginRequest() count", 1, Driver.beginRequestCount.get()); + Assert.assertEquals("Connection.endRequest() count", 1, Driver.endRequestCount.get()); + } + + +} diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java index 2c614e74a186..eff97401a3e9 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java @@ -315,6 +315,12 @@ public int getNetworkTimeout() throws SQLException { return 0; } + public void beginRequest() { + Driver.beginRequestCount.incrementAndGet(); + } + public void endRequest() { + Driver.endRequestCount.incrementAndGet(); + } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Driver.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Driver.java index 7e0bb8d19f60..2e601c0f611b 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Driver.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Driver.java @@ -30,6 +30,9 @@ public class Driver implements java.sql.Driver { public static final AtomicInteger connectCount = new AtomicInteger(0); public static final AtomicInteger disconnectCount = new AtomicInteger(0); + public static final AtomicInteger beginRequestCount = new AtomicInteger(0); + public static final AtomicInteger endRequestCount = new AtomicInteger(0); + public static void reset() { connectCount.set(0); disconnectCount.set(0); From 71d1f97a3e02763c432937d5cb42b45253718960 Mon Sep 17 00:00:00 2001 From: Fernanda Meheust Date: Mon, 6 Nov 2023 15:34:47 +0100 Subject: [PATCH 2/3] Update comment --- .../main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index 24f4c479ff75..8f6ff189e4ce 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -843,7 +843,7 @@ private boolean hasRequestBoundaryMethods(Connection connection) { beginRequest = connection.getClass().getMethod("beginRequest"); endRequest = connection.getClass().getMethod("endRequest"); } catch (NoSuchMethodException ex) { - // begin and end request not implemented, will not be invoked + // begin and end request not implemented, ignore exception } finally { requestBoundaryMethodsInitialised = true; } From 18190aaf6fa6b3af190d5576754774c943f8ef72 Mon Sep 17 00:00:00 2001 From: Fernanda Meheust Date: Tue, 7 Nov 2023 09:39:44 +0100 Subject: [PATCH 3/3] Static initialisation using Connection interface --- .../tomcat/jdbc/pool/ConnectionPool.java | 45 +++++++++---------- .../jdbc/test/ConnectionBoundariesTest.java | 14 +++++- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index 8f6ff189e4ce..8d1b1d901220 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -143,9 +143,13 @@ public class ConnectionPool { /** * Request boundaries */ - private volatile boolean requestBoundaryMethodsInitialised = false; - private Method beginRequest = null; - private Method endRequest = null; + private static final Method beginRequest; + private static final Method endRequest; + + static { + beginRequest = getConnectionMethod("beginRequest"); + endRequest = getConnectionMethod("endRequest"); + } //=============================================================================== // PUBLIC METHODS @@ -352,8 +356,8 @@ protected Connection setupConnection(PooledConnection con) throws SQLException { } else { connection = (Connection)proxyClassConstructor.newInstance(new Object[] {handler}); } - Connection underlyingConnection = con.getConnection(); - if (hasRequestBoundaryMethods(underlyingConnection)) { + if (beginRequest != null) { + Connection underlyingConnection = con.getConnection(); try { beginRequest.invoke(underlyingConnection); } catch (InvocationTargetException | IllegalAccessException ex) { @@ -830,25 +834,18 @@ protected PooledConnection createConnection(long now, PooledConnection notUsed, } /** - * If request boundaries are not initialised, checks if beginRequest and - * endRequest methods are implemented on connection object. - * - * @param connection The connection - * @return Returns true if connection is not null and connection implements - * JDBC 4.3 beginRequest and endRequest methods + * Uses reflection to get the method form the Connection interface + * @param methodName name of the method to get + * @return the method if it exists, otherwise null */ - private boolean hasRequestBoundaryMethods(Connection connection) { - if (!requestBoundaryMethodsInitialised && connection != null) { - try { - beginRequest = connection.getClass().getMethod("beginRequest"); - endRequest = connection.getClass().getMethod("endRequest"); - } catch (NoSuchMethodException ex) { - // begin and end request not implemented, ignore exception - } finally { - requestBoundaryMethodsInitialised = true; - } + private static Method getConnectionMethod(String methodName) { + Method method = null; + try { + method = Connection.class.getMethod(methodName); + } catch (NoSuchMethodException ex) { + // Ignore exception and set both methods to null } - return (connection != null && beginRequest != null && endRequest != null); + return method; } /** @@ -1066,8 +1063,8 @@ protected void returnConnection(PooledConnection con) { con.clearWarnings(); con.setStackTrace(null); con.setTimestamp(System.currentTimeMillis()); - Connection underlyingConnection = con.getConnection(); - if (hasRequestBoundaryMethods(underlyingConnection)) { + if (endRequest != null) { + Connection underlyingConnection = con.getConnection(); try { endRequest.invoke(underlyingConnection); } catch (InvocationTargetException | IllegalAccessException ex) { diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java index e9b2e98e23b2..29f13570d17a 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java @@ -40,7 +40,8 @@ public org.apache.tomcat.jdbc.pool.DataSource createDefaultDataSource() { } @Test public void connectionWithRequestBoundariesTest() throws Exception { - datasource.getPoolProperties().getDbProperties().setProperty("requestBoundaries", "true"); + double javaVersion = Double.valueOf(System.getProperty("java.class.version")); + org.junit.Assume.assumeTrue(javaVersion >= 53); Connection connection = datasource.getConnection(); Assert.assertEquals("Connection.beginRequest() count", 1, Driver.beginRequestCount.get()); Assert.assertEquals("Connection.endRequest() count", 0, Driver.endRequestCount.get()); @@ -49,5 +50,16 @@ public void connectionWithRequestBoundariesTest() throws Exception { Assert.assertEquals("Connection.endRequest() count", 1, Driver.endRequestCount.get()); } + @Test + public void connectionWithoutRequestBoundariesTest() throws Exception { + double javaVersion = Double.valueOf(System.getProperty("java.class.version")); + org.junit.Assume.assumeTrue(javaVersion < 53); + Connection connection = datasource.getConnection(); + Assert.assertEquals("Connection.beginRequest() count", 0, Driver.beginRequestCount.get()); + Assert.assertEquals("Connection.endRequest() count", 0, Driver.endRequestCount.get()); + connection.close(); + Assert.assertEquals("Connection.beginRequest() count", 0, Driver.beginRequestCount.get()); + Assert.assertEquals("Connection.endRequest() count", 0, Driver.endRequestCount.get()); + } }