Skip to content

Resolve issue where pooled connections could leak if exception occurred #167

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

Merged
merged 1 commit into from
Apr 22, 2016

Conversation

jakewins
Copy link
Contributor

during close.

  • close in PooledConnection calls reset and then returns the session to
    the pool. However, returning the session was not in a finally clause,
    so reset throwing an exception would leak the connection, eventually
    draining the pool from available sessions.

}
catch (Exception ex)
{
dispose();
}
finally
{
release.accept( this );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the code, it looks like release.accept can fail with at least an IllegalStateException. Now this doesn't have a catch protecting it, what happens if/when this is thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll bubble up to the application layer - but this is good, if the pool rejects the connection return, that's a fatal problem that should get reported to the user. IllegalStateException happens whenever the pool state is corrupted, so this is good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, 👍

during close.

- close in PooledConnection calls reset and then returns the session to
  the pool. However, returning the session was not in a finally clause,
  so reset throwing an exception would leak the connection, eventually
  draining the pool from available sessions.
@technige
Copy link
Contributor

LGTM

@jakewins jakewins merged commit e523912 into neo4j:1.0 Apr 22, 2016
@jakewins jakewins deleted the 1.0-pool-close branch April 22, 2016 12:38
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

Successfully merging this pull request may close these issues.

2 participants