This is an automated email from the ASF dual-hosted git repository.
lihan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 77dcb33743 Fix the inconsistent state of ConnectionState
77dcb33743 is described below
commit 77dcb33743d394e158aae1d538f94aaed1edd6ae
Author: lihan <li...@apache.org>
AuthorDate: Thu Aug 3 10:11:36 2023 +0800
Fix the inconsistent state of ConnectionState
---
.../jdbc/pool/interceptor/ConnectionState.java | 27 +++++---
.../tomcat/jdbc/test/TestConnectionState.java | 72 ++++++++++++++++++++++
webapps/docs/changelog.xml | 9 +++
3 files changed, 101 insertions(+), 7 deletions(-)
diff --git
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ConnectionState.java
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ConnectionState.java
index dac5463c8f..fdc64fba3b 100644
---
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ConnectionState.java
+++
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ConnectionState.java
@@ -155,14 +155,27 @@ public class ConnectionState extends JdbcInterceptor {
}
}
- result = super.invoke(proxy, method, args);
- if (read || write) {
- switch (index) {
- case 0:{autoCommit = (Boolean) (read?result:args[0]); break;}
- case 1:{transactionIsolation = (Integer)(read?result:args[0]);
break;}
- case 2:{readOnly = (Boolean)(read?result:args[0]); break;}
- case 3:{catalog = (String)(read?result:args[0]); break;}
+ try {
+ result = super.invoke(proxy, method, args);
+ if (read || write) {
+ switch (index) {
+ case 0:{autoCommit = (Boolean) (read?result:args[0]);
break;}
+ case 1:{transactionIsolation =
(Integer)(read?result:args[0]); break;}
+ case 2:{readOnly = (Boolean)(read?result:args[0]); break;}
+ case 3:{catalog = (String)(read?result:args[0]); break;}
+ }
+ }
+ } catch (Throwable e) {
+ if (write) {
+ log.warn("Reset state to null as an exception occurred while
calling method[" + name + "].", e);
+ switch (index) {
+ case 0:{autoCommit = null; break;}
+ case 1:{transactionIsolation = null; break;}
+ case 2:{readOnly = null; break;}
+ case 3:{catalog = null; break;}
+ }
}
+ throw e;
}
return result;
}
diff --git
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestConnectionState.java
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestConnectionState.java
index daa8ea4967..73e1044b9f 100644
---
a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestConnectionState.java
+++
b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestConnectionState.java
@@ -17,13 +17,19 @@
package org.apache.tomcat.jdbc.test;
import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+import org.apache.tomcat.jdbc.test.driver.Driver;
import org.junit.Assert;
import org.junit.Test;
import org.apache.tomcat.jdbc.pool.DataSourceProxy;
import org.apache.tomcat.jdbc.pool.interceptor.ConnectionState;
+import javax.sql.PooledConnection;
+
public class TestConnectionState extends DefaultTestCase {
@Test
@@ -79,4 +85,70 @@ public class TestConnectionState extends DefaultTestCase {
c1 = d1.getConnection();
Assert.assertEquals("Catalog should be
information_schema",c1.getCatalog(),"information_schema");
}
+
+ @Test
+ public void testWithException() throws SQLException {
+ DriverManager.registerDriver(new MockErrorDriver());
+ // use our mock driver
+ datasource.setDriverClassName(MockErrorDriver.class.getName());
+ datasource.setUrl(MockErrorDriver.url);
+ datasource.setDefaultAutoCommit(true);
+ datasource.setMinIdle(1);
+ datasource.setMaxIdle(1);
+ datasource.setMaxActive(1);
+ datasource.setJdbcInterceptors(ConnectionState.class.getName());
+ Connection connection = datasource.getConnection();
+ PooledConnection pc = (PooledConnection) connection;
+ MockErrorConnection c1 = (MockErrorConnection) pc.getConnection();
+ Assert.assertTrue("Auto commit should be true", c1.getAutoCommit());
+ connection.close();
+ c1.setThrowError(true);
+ Connection c2 = datasource.getConnection();
+ try {
+ c2.setAutoCommit(false);
+ } catch (SQLException e) {
+ // ignore
+ }
+ Assert.assertFalse("Auto commit should be false", c2.getAutoCommit());
+ c2.close();
+ DriverManager.deregisterDriver(new MockErrorDriver());
+ }
+ public static class MockErrorDriver extends Driver {
+ @Override
+ public Connection connect(String url, Properties info) throws
SQLException {
+ return new MockErrorConnection(info);
+ }
+ }
+
+ public static class MockErrorConnection extends
org.apache.tomcat.jdbc.test.driver.Connection {
+ private boolean throwError = false;
+ private boolean autoCommit = false;
+ public void setThrowError(boolean throwError) {
+ this.throwError = throwError;
+ }
+
+ public MockErrorConnection(Properties info) {
+ super(info);
+ }
+
+ @Override
+ public boolean getAutoCommit() throws SQLException {
+ return autoCommit;
+ }
+
+ @Override
+ public void close() throws SQLException {
+ super.close();
+ throwError = true;
+ }
+
+ @Override
+ public void setAutoCommit(boolean autoCommit) throws SQLException {
+ this.autoCommit = autoCommit;
+ if (throwError) {
+ throw new SQLException("Mock error");
+ }
+ }
+
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e270f713da..cf09e49a8a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -162,6 +162,15 @@
</fix>
</changelog>
</subsection>
+ <subsection name="jdbc-pool">
+ <changelog>
+ <fix>
+ Fix the <code>ConnectionState</code> state will be inconsistent with
actual
+ state on the connection when an exception occurs while writing. Pull
request
+ <pr>643</pr> provided by Wenjun Xiao. (lihan)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Other">
<changelog>
<update>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
