Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

com.zaxxer.hikari.pool.ProxyConnection$ClosedConnection should return false for isClosed() instead of throwing an exception #1141

Closed
tomdcc opened this issue May 4, 2018 · 3 comments
Assignees
Labels

Comments

@tomdcc
Copy link

tomdcc commented May 4, 2018

Environment

HikariCP version: 3.1.0
JDK version     : 1.8.0_111
Database        : PostgreSQL
Driver version  : 42.1.4

See https://github.com/brettwooldridge/HikariCP/blob/HikariCP-3.1.0/src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java#L478

It's not possible to check if a connection is closed by calling isClosed() without special exception handling as Hikari throws an exception. There should be a check for the isClosed method in the handler that should just return false.

@brettwooldridge
Copy link
Owner

brettwooldridge commented May 9, 2018

@tomdcc Is that true? The implementation of isClosed() seems correct (and does not throw). We have a test that validates that the behavior is correct.

Are you possibly calling isClosed() on a Connection that you have unwrapped via unwrap() after closing the Connection?

@tomdcc
Copy link
Author

tomdcc commented May 14, 2018

Sorry for the delay in replying, I haven't had time to try to recreate it, and the code that was triggering this has changed somewhat so it's not straight forward.

I could have sworn that the reported stack trace showed the exception being thrown by ClosedConnection. That code was doing some unwrapping at one point, though.

Hmm. It does look like this sequence might do it, though I haven't verified it:

Connection con = proxyConnection.unwrap(Connection.class);
// ...
con.isClosed();

This is simplified, the code as it stands doesn't make much sense. The unwrapping was happening away from the isClosed() call.

Caller expects to get an unwrapped connection for some reason, but gets the ClosedConnection delegate instead. They then call isClosed() and sadness ensues.

@brettwooldridge
Copy link
Owner

We can certainly add isClosed() to the interceptor. While an unusual pattern, it still should not throw an exception.

kollstrom pushed a commit to kollstrom/HikariCP that referenced this issue Feb 4, 2021
…losed() or close() on a already closed Connection, as per JDBC specification.
kollstrom pushed a commit to kollstrom/HikariCP that referenced this issue Feb 4, 2021
…losed() or close() on a already closed Connection, as per JDBC specification.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants