Fix OPENDJ-1156 - NPE in ReferenceCountedObject after shutting down directory

Matthew Swift

28.20.2013 3d8848801b06656f425b027d16219b026bbbb2a8

Fix OPENDJ-1156 - NPE in ReferenceCountedObject after shutting down directory

* ensure that resources associated with an LDAP connection are only released at most once, when the client closes the connection. Previously resources were released when a connection error was detected as well
* minor fix for ConnectionState where a "close" state transition is lost when closing is performed re-entrantly from within a ConnectionEventListener error notification.

 opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/ConnectionState.java

@@ -98,10 +98,10 @@

            @Override
            boolean notifyConnectionClosed(final ConnectionState cs) {
                cs.state = CLOSED;
                for (final ConnectionEventListener listener : cs.listeners) {
                    listener.handleConnectionClosed();
                }
                cs.state = CLOSED;
                return true;
            }

@@ -111,11 +111,15 @@
                // Transition from valid to error state.
                cs.failedDueToDisconnect = isDisconnectNotification;
                cs.connectionError = error;
                cs.state = ERROR;
                /*
                 * FIXME: a re-entrant close will invoke close listeners before
                 * error notification has completed.
                 */
                for (final ConnectionEventListener listener : cs.listeners) {
                    // Use the reason provided in the disconnect notification.
                    listener.handleConnectionError(isDisconnectNotification, error);
                }
                cs.state = ERROR;
                return true;
            }

@@ -156,10 +160,10 @@

            @Override
            boolean notifyConnectionClosed(final ConnectionState cs) {
                cs.state = ERROR_CLOSED;
                for (final ConnectionEventListener listener : cs.listeners) {
                    listener.handleConnectionClosed();
                }
                cs.state = ERROR_CLOSED;
                return true;
            }
        },

 opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java

@@ -295,7 +295,6 @@
    public void close(final UnbindRequest request, final String reason) {
        // FIXME: I18N need to internationalize this message.
        Validator.ensureNotNull(request);

        close(request, false, Responses.newResult(ResultCode.CLIENT_SIDE_USER_CANCELLED)
                .setDiagnosticMessage(reason != null ? reason : "Connection closed by client"));
    }
@@ -624,10 +623,10 @@
        }

        /*
         * Now try cleanly closing the connection if possible. Only send unbind
         * if specified.
         * If this is the final client initiated close then release close the
         * connection and release resources.
         */
        if (unbindRequest != null) {
        if (notifyClose) {
            final ASN1BufferWriter asn1Writer = ASN1BufferWriter.getWriter();
            try {
                ldapWriter.unbindRequest(asn1Writer, nextMsgID.getAndIncrement(), unbindRequest);
@@ -640,10 +639,10 @@
            } finally {
                asn1Writer.recycle();
            }
        }
        factory.getTimeoutChecker().removeConnection(this);
        connection.closeSilently();
        factory.releaseTransportAndTimeoutChecker();
        }

        // Notify listeners.
        if (tmpListeners != null) {

 opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/ConnectionStateTest.java

@@ -29,6 +29,7 @@
import static org.forgerock.opendj.ldap.ErrorResultException.newErrorResult;
import static org.forgerock.opendj.ldap.responses.Responses.newGenericExtendedResult;
import static org.mockito.Matchers.same;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -37,6 +38,8 @@
import org.forgerock.opendj.ldap.ErrorResultException;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.responses.ExtendedResult;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.testng.annotations.Test;

/**
@@ -247,4 +250,28 @@
        assertThat(state.isClosed()).isFalse();
        assertThat(state.getConnectionError()).isNull();
    }

    /**
     * Tests that reentrant close from error listener is handled.
     */
    @Test
    public void testReentrantClose() {
        final ConnectionState state = new ConnectionState();
        final ConnectionEventListener listener1 = mock(ConnectionEventListener.class);
        doAnswer(new Answer<Void>() {
            public Void answer(InvocationOnMock invocation) {
                state.notifyConnectionClosed();
                return null;
            }
        }).when(listener1).handleConnectionError(false, ERROR);

        state.addConnectionEventListener(listener1);
        state.notifyConnectionError(false, ERROR);

        assertThat(state.isValid()).isFalse();
        assertThat(state.isClosed()).isTrue();
        assertThat(state.getConnectionError()).isSameAs(ERROR);
        verify(listener1).handleConnectionError(false, ERROR);
        verify(listener1).handleConnectionClosed();
    }
}