GoogleCloudPlatform / GoogleCloudPlatform/alloydb-java-connector

ConnectionSocket leaks an established socket when the TLS handshake or metadata exchange fails

Open
#739 0 comments 0 reactions 1 assignee Claimed by @enocom View on GitHub
priority: p2 type: bug
Dominant language
Java
Stars
20
Forks
9
Avg merge
9d 8h
Merged PRs (30d)
3

Description

While working on built-in metrics, I discovered a small bug we should fix.

ConnectionSocket.connect() builds an SSLSocket, connects it, completes the TLS handshake, then runs the metadata exchange. Two failure paths throw past a fully established socket without closing it:

- ConnectionSocket.java:126 — catch (IOException e) { logger.debug("TLS handshake failed!"); throw e; }
- ConnectionSocket.java:259 — throw new MetadataExchangeException(...)

Connector.connect catches these, records a metric, calls forceRefresh(), and rethrows. It never holds a reference it could close, so the socket is abandoned.

The UserConfigException path (no address for the requested IP type) throws before socket.connect(...), so nothing is established there and it needs no change.

This is not an unbounded FD leak — verified on JDK 21 that NioSocketImpl registers a cleaner and abandoned sockets release their FDs on GC. The problem is that release is tied to GC, which is driven by heap pressure rather than FD pressure. A Socket object is tiny, so an application in a dial-failure loop can reach ulimit -n with a nearly empty heap and no GC pending.

Two things make the metadata-exchange path a problem:

1. The socket is a fully established, TLS-authenticated connection abandoned mid-protocol. No FIN is sent, so the instance holds a connection slot in ESTABLISHED until GC runs or TCP keepalive reaps it. setKeepAlive(true) is set, but the default tcp_keepalive_time is around two hours.
2. Metadata-exchange failures are usually persistent config errors — wrong auth type, expired or invalid IAM token, caller lacks instance access — rather than transient. A connection pool retries on its normal interval, and each retry establishes and abandons another TCP+TLS connection plus triggers a forceRefresh() API call.

We should do a best-effort close on both paths in ConnectionSocket.connect().

Once built-in metrics land (#711), the signature is dial_count{status="mdx_error"} climbing while open_connections stays flat.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.