Uploaded image for project: 'Agroal'
  1. Agroal
  2. AG-165

XAConnection.close is never called

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • Major
    • 1.11
    • 1.9
    • pool
    • None
    • False
    • False
    • Undefined
    • Hide

      I've built a repro in Quarkus that shows that two PostgreSQL drivers (mainstream pgjdbc & alternate pgjdbc-ng) never have their XA connections closed; instead they are leaked. The mainstream pgjdbc driver implements `finalize` and cleans up the connections that way with no warnings/logs. The alternate pgjdbc-ng driver cleans up the connections via phantom reference queues; this driver logs warnings about the leaks as well.

       

      Running the repo tests (via `./gradlew test`) which the pgjdbc-ng driver you will see leaks logged.

       

      Debugging the tests with either driver will allow you to breakpoint/breaklog the `close` method of either driver's physical connection implementation (`PgConnection` for pgjdbc / `PGDirectConnection` for pgjdbc-ng) and see that the close is only ever called in finalize or via phantom reference cleanup.

      Show
      I've built a repro in Quarkus that shows that two PostgreSQL drivers (mainstream pgjdbc & alternate pgjdbc-ng) never have their XA connections closed; instead they are leaked. The mainstream pgjdbc driver implements `finalize` and cleans up the connections that way with no warnings/logs. The alternate pgjdbc-ng driver cleans up the connections via phantom reference queues; this driver logs warnings about the leaks as well.   Running the repo tests (via `./gradlew test`) which the pgjdbc-ng driver you will see leaks logged.   Debugging the tests with either driver will allow you to breakpoint/breaklog the `close` method of either driver's physical connection implementation (`PgConnection` for pgjdbc / `PGDirectConnection` for pgjdbc-ng) and see that the close is only ever called in finalize or via phantom reference cleanup.

    Description

      The constructor of `io.agroal.pool.ConnectionHandler(XAConnection, Pool)` releases the `XAConnection` handed to it after obtaining a logical `Connection` (via `XAConnection.getConnection()`) and an `XAResource` (via `XAConnection.getXAResource()`). This essentially means that the `XAConnection` itself can never be closed properly.

      According to the JDBC spec/docs the `XAConnection` must be closed directly when the pool/middleware has determined the physical connection is no longer needed; this is because an `XAConnection` is also a `PooledConnection`. Both interfaces share a common `getConnection()` method which the docs say should return a connection handle that does not close the connection but instead notifies it's listeners of the close event which the pool/middleware can listen to and decide if the physical connection should be closed.

      `ConnectionHandler` only keeps a reference to the connection handle and therefore only closes the connection handle. It also never registered a listener on the `XAConnection` that it can use to be notified of connection close events.

      The pool cannot rely upon finalize and/or leak detection to close physical database connections because under heavy load these facilities may be delayed by a significant amount of time which in turn can cause connection starvation issues on the server (e.g. too many open connections).

      Attachments

        Activity

          People

            lbarreiro-1 Luis Barreiro
            kevinwooten Kevin Wooten (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: