vibur / vibur-dbcp

Vibur DBCP - concurrent and dynamic JDBC connection pool
http://www.vibur.org
Apache License 2.0
96 stars 11 forks source link

Statement leak when statement cache is disabled #7

Closed ato closed 8 years ago

ato commented 8 years ago

StatementInvocationHandler intercepts the close() method and unless the statement cache is enabled it appears to never close the wrapped statement object:

https://github.com/vibur/vibur-dbcp/blob/a893a63/src/main/java/org/vibur/dbcp/proxy/StatementInvocationHandler.java#L93

Could this be a regression introduced by the refactoring in commit e5e4116f8cbdb548a86507ef00b0e9734dbb09ef? Prior to that commit there was an else branch in processClose():

if (statement.state() != null) { // if this statement is in the cache
    ... mark available in cache and close if already evicted ...
} else
    closeStatement(rawStatement);

I ran into this when debugging an OutOfMemoryError with the Oracle JDBC driver (which internally holds onto a list of all open statements). Enabling statement cache with setStatementCacheMaxSize seems to be a viable workaround as the statement cache will close the wrapped statement on eviction.

simeonmalchev commented 8 years ago

@ato, thanks for reporting this.

Most likely is a regression as you suggested. Will try to fix it and push a new release out as soon as I can. In the mean time, the workaround that you suggested is completely viable, yes.

simeonmalchev commented 8 years ago

This is now fixed on master. Will try to push out a new release to maven central sometime tomorrow.

ato commented 8 years ago

Awesome. Thanks! :-)

simeonmalchev commented 8 years ago

Just pushed version 8.0 to maven central. Should be available there soon.

simeonmalchev commented 8 years ago

/cc @dcalde