PGXAConnection of Driver Version 9.3.1001 does not play nicely with Apache DBCP 1.4

[ctolkmit]

Using the PGXAConnection with Apache DBCP 1.4 results in something like the following:

java.sql.SQLException: Connection does not have a registered XAResource jdbc:postgresql://tng00.pgsql.tng.de:5432/prospero_test?loginTimeout=0&socketTimeout=0&prepareThreshold=5&unknownLength=0&tcpKeepAlive=false&binaryTransfer=false, UserName=prospero_test_user, PostgreSQL Native Driver
        at org.apache.commons.dbcp.managed.TransactionRegistry.getXAResource(TransactionRegistry.java:78)
        at org.apache.commons.dbcp.managed.TransactionContext.setSharedConnection(TransactionContext.java:88)
        at org.apache.commons.dbcp.managed.ManagedConnection.updateTransactionStatus(ManagedConnection.java:131)
        at org.apache.commons.dbcp.managed.ManagedConnection.<init>(ManagedConnection.java:55)
        at org.apache.commons.dbcp.managed.ManagedDataSource.getConnection(ManagedDataSource.java:77)

The problem lies within getConnection() where a new Connection Proxy is returned every time the method is called. Since DBCP uses the Connection Object has a key to a (weak) hash map, however hashCode() and equals() do not always deliver stable data on the returned proxies.

We are currently using a patched version, with the following things added:

public class PGXAConnection // code omitted ...

    private static final Method OBJECT_EQUALS =
            getObjectMethod("equals", Object.class);

    private static final Method OBJECT_HASHCODE =
            getObjectMethod("hashCode");

    // code omitted ,,,

    private static Method getObjectMethod(final String name, final Class... types) {
        try {
            // null 'types' is OK.
            return Object.class.getMethod(name, types);
        } catch (final NoSuchMethodException e) {
            throw new IllegalArgumentException(e);
        }
    }

    // code omitted ...

    private class ConnectionHandler implements InvocationHandler
    {
        // code omitted ...

       // this is my new implementation of the invoke-Method:
        @Override
        public Object invoke(final Object proxy, final Method method, final Object[] args)
        throws Throwable
        {
            if (OBJECT_HASHCODE.equals(method)) {
                // System.out.println("hashCode: " + proxy + " -> " +
                // con.hashCode());
                return con == null ? 0 : con.hashCode();
            }

            if (OBJECT_EQUALS.equals(method)) {
                return equalsInternal(proxy, args[0]);
            }

            // System.out.println("call to method: " + method);

        if (state != STATE_IDLE)
            {
                final String methodName = method.getName();
                if (methodName.equals("commit") ||
                    methodName.equals("rollback") ||
                    methodName.equals("setSavePoint") ||
                    (methodName.equals("setAutoCommit") && ((Boolean) args[0]).booleanValue()))
                {
            throw new PSQLException(GT.tr("Transaction control methods setAutoCommit(true), commit, rollback and setSavePoint not allowed while an XA transaction is active."),
                        PSQLState.OBJECT_NOT_IN_STATE);
                }
            }
            try {
                return method.invoke(con, args);
            } catch (final InvocationTargetException ex) {
                throw ex.getTargetException();
            }
        }

        // this will provide the actual internal comparison
        private boolean equalsInternal(final Object proxy, final Object other) {
            // System.out.println("equalsInternal: " + proxy + " ?= " + other);
            if (other == null) {
                return false;
            }
            if (Proxy.isProxyClass(other.getClass())) {
                final InvocationHandler otherHandler = Proxy.getInvocationHandler(other);
                if (otherHandler instanceof ConnectionHandler) {
                    final ConnectionHandler otherConnectionHandler = (ConnectionHandler) otherHandler;
                    return con.equals(otherConnectionHandler.con);
                } else {
                    return false;
                }
            } else {
                // other is not a proxied object
                return con.equals(other);
            }
        }

        // code omitted ...
    }

    // code omitted ...

}

I know I should better provide patch or pull-request, but my editor currently does a lot of automatic code-formatting resulting in a patch that nearly changes the entire file.

I hope I could help anyways.

Regards
Carsten

[alexismeneses]

Hi,

It seems that this has already been fixed into the master (commit 8d74338 PR #125), at least for the equals() part.

Is there a point returning 0 on hashCode() when con is null?

Regards

[ctolkmit]

Not sure right now about the 0 if con is null. It's just a safety pattern I am used to type automatically ;)

[ctolkmit]

Oh, and yes, I saw the equals()-Part in the Master, however the hashCode() part is also important.
And I don't like branching from an unreleased version, and we are currently using my patched version in production.

[alexismeneses]

Unless the zero-if-null thing is useful for a case that might happen and for which we can write a test, I think the issue is fixed.

Letting hashCode() fall into method.invoke(con, args) will do the job as two proxies with the same underlying connection will return the same consistent value.

The only remaining question is: should 8d74338 be backported to older branches?

[davecramer]

Yes, it probably should be backported.

Dave Cramer

On 1 July 2014 06:48, Alexis Meneses notifications@github.com wrote:

Unless the zero-if-null thing is useful for a case that might happen
and for which we can write a test, I think the issue is fixed.

Letting hashCode() fall into method.invoke(con, args) will do the job as
two proxies with the same underlying connection will return the same
consistent value.

The only remaining question is: should 8d74338
8d74338
be backported to older branches?

—
Reply to this email directly or view it on GitHub
#162 (comment).

[davecramer]

historic, if this still exists in the latest driver please advise