Bug 61545 - ProxyConnection.invoke() does not handle javax.sql.PooledConnection method calls
Summary: ProxyConnection.invoke() does not handle javax.sql.PooledConnection method calls
Status: RESOLVED FIXED
Alias: None
Product: Tomcat Modules
Classification: Unclassified
Component: jdbc-pool (show other bugs)
Version: unspecified
Hardware: PC Mac OS X 10.1
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-20 08:57 UTC by Nils Winkler
Modified: 2017-09-22 07:43 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nils Winkler 2017-09-20 08:57:35 UTC
I've found an issue with tomcat-jdbc and XA connections, where Tomcat Pool's ProxyConnection class does not handle invocations done using the javax.sql.PooledConnection interface.

I'm seeing this with Tomcat v7.0.78, but the same code is used in Tomcat's 8.5 codebase. My use case involves the following setup:

* Database: MySQL, PostgreSQL or Oracle - same issue on all three of them.
* Tomcat Pool is used to define a context-specific XADataSource to be used by the web application.
* The web application uses Spring for setup, and Jencks as an XA transaction manager.
* Jencks uses tranql-connector under the hood (version 1.7 or 1.8 show the same behavior).

The issue can be seen from Tranql's code: http://grepcode.com/file/repo1.maven.org/maven2/org.tranql/tranql-connector/1.8/org/tranql/connector/jdbc/ManagedXAConnection.java?av=f#55

Here, tranql gets an XAConnection from the pool (Tomcat Pool in this case) and tries to set a ConnectionEventListener on the connection:

       this.xaConnection = xaConnection;
         xaConnection.addConnectionEventListener(new ConnectionEventListener() {
                     public void connectionClosed(ConnectionEvent event) {
                 //we should be handling this independently
             }
 
                     public void connectionErrorOccurred(ConnectionEvent event) {
                 Exception e = event.getSQLException();
                 unfilteredConnectionError(e);
             }
         });

The xaConnection.addConnectionEventListener gets propagated through the JdbcInterceptor hierarchy in Tomcat Pool, all the way to the ProxyConnection.invoke() method: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java?view=markup#l92

Since the addConnectionEventListener method is defined on javax.sql.PooledConnection, none of the if-clauses in the ProxyConnection.invoke() method match and the code finally ends up in line 126, where the "addConnectionEventListener" is invoked on the java.sql.Connection instance, which of course does not implement the javax.sql.PooledConnection interface: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java?view=markup#l126

PooledConnection poolc = connection;
if (poolc!=null) {
    return method.invoke(poolc.getConnection(),args);
} else {
    throw new SQLException("Connection has already been closed.");
}

The error that is shown looks like this:

Caused by: java.lang.IllegalArgumentException: object is not an instance of declaring class
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at org.apache.tomcat.jdbc.pool.ProxyConnection.invoke(ProxyConnection.java:126)
	at org.apache.tomcat.jdbc.pool.JdbcInterceptor.invoke(JdbcInterceptor.java:109)
	at org.apache.tomcat.jdbc.pool.interceptor.AbstractCreateStatementInterceptor.invoke(AbstractCreateStatementInterceptor.java:80)
	at org.apache.tomcat.jdbc.pool.JdbcInterceptor.invoke(JdbcInterceptor.java:109)
	at org.apache.tomcat.jdbc.pool.interceptor.AbstractCreateStatementInterceptor.invoke(AbstractCreateStatementInterceptor.java:80)
	at org.apache.tomcat.jdbc.pool.JdbcInterceptor.invoke(JdbcInterceptor.java:109)
	at org.apache.tomcat.jdbc.pool.interceptor.ConnectionState.invoke(ConnectionState.java:153)
	at org.apache.tomcat.jdbc.pool.JdbcInterceptor.invoke(JdbcInterceptor.java:109)
	at org.apache.tomcat.jdbc.pool.TrapException.invoke(TrapException.java:41)
	... 111 more

In the case of Oracle, the "poolc.getConnection()" call returns an instance of oracle.jdbc.driver.LogicalConnection, which does not implement javax.sql.PooledConnection.

Since javax.sql.XAConnection extends the javax.sql.PooledConnection interface, the reason for the error stems from the check in line 106 of ProxyConnection.java: http://svn.apache.org/viewvc/tomcat/trunk/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java?view=markup#l106

} else if (method.getDeclaringClass().equals(XAConnection.class)) {
    try {
        return method.invoke(connection.getXAConnection(),args);
    }catch (Throwable t) {

The code does the right thing (using the "connection.getXAConnection()" instance to invoke calls that are supposed to be done on an XAConnection, but it fails to account for the fact that javax.sql.XAConnection also extends the javax.sql.PooledConnection interface. Due to this incomplete check, the call to javax.sql.PooledConnection.addConnectionEventListener() is not detected here, but falls through to the call in line 126.

The code in line 106 should be changed to not only check for calls made to XAConnection, but also include methods defined in super-classes (or super-interfaces) of XAConnection:

} else if (method.getDeclaringClass().isAssignableFrom(XAConnection.class)) {


Using "isAssignableFrom" instead of "equals" accomplishes that.

Sorry for the long description - let me know if there are any questions or require a unit test case and/or a patch for fixing this issue.
Comment 1 Mark Thomas 2017-09-22 07:23:38 UTC
Thanks for the report, detailed explanation and fix.

Fixed in:
- trunk for 9.0.0 onwards
- 8.5.x for 8.5.22 onwards
- 8.0.x for 8.0.47 onwards
- 7.0.x for 7.0.82 onwards
Comment 2 Nils Winkler 2017-09-22 07:43:29 UTC
Thanks for the quick fix! Happy to help!