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

Connection leak in ConnectionFactoryImpl#tryConnect #2350

Closed
wyrzyk opened this issue Nov 22, 2021 · 3 comments
Closed

Connection leak in ConnectionFactoryImpl#tryConnect #2350

wyrzyk opened this issue Nov 22, 2021 · 3 comments

Comments

@wyrzyk
Copy link
Contributor

wyrzyk commented Nov 22, 2021

ConnectionFactoryImpl#tryConnect creates a new PGStream and establishes connection with the database, later it calls ConnectionFactoryImpl#doAuthentication. If ConnectionFactoryImpl#doAuthentication fails with an exception (for example 'too many connections for role'), then we can't use the connection, because PGConnections doesn't have queryExecutor and the socket is not closed. The socket is left to be closed by a Finalizer.
image

Driver Version?

42.2.14

Java Version?

1.8

OS Version?

Ubuntu

PostgreSQL Version?

Aurora 10.14

To Reproduce

I run a stress test on an app and reach a connection saturation on the database site. After the test, the number of connections remained high. I took a heap dump from the application and saw that there's not many PGConnections, but the app kept hundreds of open SocksSocketImpl. The connections were accessible only by the finalizer, but the finalizer was not processing them (the sockets were not enqueued yet.).

I tried to reproduce it with a minimal, reproducible example, but it seems it would need to push both GC and DB to reproduce the issue (socket connections going to old gen and not being closed for a longer period).

Expected behaviour

I'd expect to close the socket as soon as possible, when we know it can't be used anymore.

And what actually happens:
Finalizer closes the socket (at some point in the future).

wyrzyk added a commit to wyrzyk/pgjdbc that referenced this issue Nov 22, 2021
When `ConnectionFactoryImpl#tryConnect` fails with an exception,
a newly created PGStream is left without closing it. The finalizer
handles the close, so it's not a problem most of the time.
But Finalizer has no guarantees on the time when it'll close
the resource. The resource may hang open for a longer period.

After the change PGStream is closed just before losing the reference.
davecramer pushed a commit that referenced this issue Nov 24, 2021
When `ConnectionFactoryImpl#tryConnect` fails with an exception,
a newly created PGStream is left without closing it. The finalizer
handles the close, so it's not a problem most of the time.
But Finalizer has no guarantees on the time when it'll close
the resource. The resource may hang open for a longer period.

After the change PGStream is closed just before losing the reference.
@davecramer
Copy link
Member

closed with PR #2351

@buitcj
Copy link

buitcj commented Jun 28, 2022

@wyrzyk SELECT pg FROM org.postgresql.jdbc.PgConnection pg WHERE (pg.queryExecutor = null) does not appear to work when I connect to the pg instance (ERROR: improper qualified name). Should this command work without any further steps? Is there any way to query the instance to tell if I'm hitting the issue that you've fixed?

@davecramer
Copy link
Member

davecramer commented Jun 28, 2022

@wyrzyk SELECT pg FROM org.postgresql.jdbc.PgConnection pg WHERE (pg.queryExecutor = null) does not appear to work when I connect to the pg instance (ERROR: improper qualified name). Should this command work without any further steps? Is there any way to query the instance to tell if I'm hitting the issue that you've fixed?

Ok, no this command should not work.

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

3 participants