swaldman / c3p0

a mature, highly concurrent JDBC Connection pooling library, with support for caching and reuse of PreparedStatements.
http://www.mchange.com/projects/c3p0
Other
1.28k stars 338 forks source link

Are JDBC 4.3 drivers required since version 0.10.x ? NoSuchMethodError: java.sql.Connection.beginRequest()V #176

Closed skarzhevskyy closed 4 months ago

skarzhevskyy commented 5 months ago

We are running app with Oracel JDBC drivers v21.13.0.0 under java 8.

after upgrade to c3p0 0.10.0 We observed error

Caused by: java.lang.NoSuchMethodError: java.sql.Connection.beginRequest()V
    at com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool$LiveRequestBoundaryMarker.attemptNotifyBeginRequest(C3P0PooledConnectionPool.java:145)
    at com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool.markBeginRequest(C3P0PooledConnectionPool.java:217)
    at com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool.checkoutPooledConnection(C3P0PooledConnectionPool.java:726)
    at com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource.getConnection(AbstractPoolBackedDataSource.java:105)

the same application (same dependencies ojdbc8_g-21.13.0.0.jar) runs runder java 11. Also there are no problem running on PostgreSQL with java 8 or 11.

The question is since you try to make c3p0 compatible with previos generations of java (even v 7) do you want want to try to make it compatible with JDBC 4.2 or this is the end of an era ?

FIY https://www.oracle.com/database/technologies/faq-jdbc.html The table lists the Oracle JDBC drivers and the JDBC specification supported in that release.

v 19.x     
      ojdbc8.jar  is JDBC 4.2
      ojdbc10.jar  is JDBC 4.3

v 21.x      and  v 3.x
      ojdbc8.jar    is JDBC 4.2
      ojdbc10.jar  is JDBC 4.3
skarzhevskyy commented 4 months ago

Hi @swaldman I think the problem reported here is realted to https://github.com/swaldman/c3p0/issues/177

The use case very simple:
Java 8 runtime + oracle.jdbc.driver and you will get java.lang.NoSuchMethodError

swaldman commented 4 months ago

Hi @skarzhevskyy!

I am very sorry I missed this. Somehow, I am not reliably getting (or at least seeing) issue notifications. I had not seen this issue before today's discussion.

I absolutely intend for c3p0 to be as backward compatible as possible, and will add handling for this NoSuchMethodError case.

I don't actually understand why it is happening though, as we check for the presence of these methods on the first checkout from every pool. Somehow with the Oracle 8 driver that test sometimes succeeds but would subsequently fail. I think I'll handle this by catching the error and reverting to not supporting begin/endRequest should the error ever occur. If you are interested in reviewing the logic or offering suggestions, it is here: https://github.com/swaldman/c3p0/blob/0.10.x/src/com/mchange/v2/c3p0/impl/C3P0PooledConnectionPool.java#L123-L218

Thank you again for the detailed report, and I am sorry for the flake.

skarzhevskyy commented 4 months ago

Hi , I can see the problem.

in findRequestBoundaryMarker line 191

 Method beginRequest = conn.getClass().getMethod("beginRequest");

code is checking for presence of beginRequest() in class T4CConnection and it is prsent in oracel v 21.13.0.0 but it does not override one from java.sql.Connection

if we are about to class this Method, bytecode will be:

    INVOKEVIRTUAL oracle/jdbc/driver/T4CConnection.beginRequest()V

the actual ivocation attemptNotifyBeginRequest.attemptNotifyBeginRequest line 145

Connection conn = acpc.getPhysicalConnection();
conn.beginRequest();

code is calling metod of interface (not of the class implemeting it)

bytecode here is:

   INVOKEINTERFACE java/sql/Connection.beginRequest()V

I will sugest solution tomorow.

skarzhevskyy commented 4 months ago

Hi Steve Since C3P0 JDBC 4.3 code relies on presence of java.sql.Connection#beginRequest suggestion is to check presence of the needed method on java.sql.Connection class as well.

The check can save results in the static member of C3P0PooledConnectionPool and it will be safe since the ClassLoader that loded C3P0 and java.sql.Connection will not change.

Thanks for supporting C3P0 project!

swaldman commented 4 months ago

@skarzhevskyy Thank you for tracing the issue, and for the suggestions!

swaldman commented 4 months ago

@skarzhevskyy Okay! I think this issue is resolved (though I don't have an Oracle back-end to test it on). The implementation is basically as you suggest. I've added support for the case where the methods are present but not in the available Connection interface by reflection.

https://github.com/swaldman/c3p0/commit/f0a87da8b6328e7a1945e958909684d0c2118755

swaldman commented 4 months ago

If you want to try it out, a SNAPSHOT release is available on repository https://www.mchange.com/repository as com.mchange:c3p0:0.10.1-SNAPSHOT

SNAPSHOTs are as they usually are unstable, overwritable, so please don't permanently depend on this. I'll try to get it into a real 0.10.1 release soon, but I'd love feedback about whether it fully addresses the issue before I do.

Thanks!

skarzhevskyy commented 4 months ago

Nice.

I did run test on todays c3p0 0.10.1-SNAPSHOT. Tests all geeen.

oracle.ojdbc8       v21.13.0.0        java 8*, java 11, java 21
oracle.ojdbc11      v21.13.0.0            n/a, java 11,
oracle.ojdbc8       v23.3.0.23.09     java 8*, java 11, java 21 
h2                  v2.2.224          java 8 , java 11, java 21
mariadb-java-client v3.3.3                n/a, java 11, java 21
postgresql          v42.7.3           java 8 , java 11, java 21

'* ojdbc8 tests on java 8 used to fail with c3p0 0.10.0

These are ORM, DDL and connection timeouts and locks tests. I don't have any Oracle Application Continuity unit tests.

swaldman commented 4 months ago

@skarzhevskyy Thank you a ton for all the testing. I really do appreciate it.

swaldman commented 4 months ago

Hi! c3p0-0.10.1 is now a release. This issue should be addressed in that version. I'll close it for now. If anything goes wrong, please feel free to submit a new issue or reopen this one. Thanks!

Thank you again for all the help and testing.