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

Closing a connection with "usageCount > 1" is possible. #16

Closed
rankinc opened this issue Mar 30, 2014 · 4 comments
Closed

Closing a connection with "usageCount > 1" is possible. #16

rankinc opened this issue Mar 30, 2014 · 4 comments

Comments

@rankinc
Copy link
Contributor

rankinc commented Mar 30, 2014

JdbcPooledConnection.close() says:

    // this should never happen, should we throw an exception or log at warn/error?
    if (usageCount > 0) {
        log.warn("close connection with usage count > 0, " + this);
    }

However, should JdbcPooledConnection.getConnectionHandle() throw a SQLException because (e.g.) testConnection() has failed, then usageCount will still have been incremented when XAPool.getConnectionHandle() tries to close the invalid connection.

The patch is trivial:

// Increment the usage count
usageCount++
try {

   // etc

} catch (SQLException e) {
    // This connection must be invalid, so revert the usage counter.
    // Note: Closing a handle with usageCount > 0 "should never happen".
    --usageCount;
    throw e;
}

Cheers,
Chris

@rankinc
Copy link
Contributor Author

rankinc commented Mar 30, 2014

I've committed this change into my BTM tree on github, but cannot see how to create a new pull request that doesn't also include my existing and outstanding pull request #15.

@vladmihalcea
Copy link

Hi Chris,

I ran into the same issues, so I decided to write a guide for Git feature branching for the Bitronix project:

http://vladmihalcea.com/2014/02/13/a-beginners-guide-to-git-feature-branches/

I hope it will be one day incorporated in the README section.

Vlad

@rankinc
Copy link
Contributor Author

rankinc commented Mar 31, 2014

Hi Vlad,
Thanks, I think that a "feature branch" is well worth using for future pull requests. However, seeing as I was actively asked to create #15, I don't think that it's unreasonable to hope that it be merged first.

Cheers,
Chris

@rankinc
Copy link
Contributor Author

rankinc commented Apr 1, 2014

Closed, and replaced with #21.

@rankinc rankinc closed this as completed Apr 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants