Backport fixes for OPENDJ-672 and OPENDJ-1156: NPE in ReferenceCountedObjec...

Matthew Swift

28.27.2013 5ed0ee0b93568f456869fc8bec58ccf45686284e

Backport fixes for OPENDJ-672 and OPENDJ-1156: NPE in ReferenceCountedObject after shutting down directory

 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;
            }
        },

 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(
                        "Connection closed by client" + (reason != null ? ": " + reason : "")));
@@ -624,25 +623,27 @@
            }
        }

        // Now try cleanly closing the connection if possible.
        // Only send unbind if specified.
        if (unbindRequest != null) {
        /*
         * If this is the final client initiated close then release close the
         * connection and release resources.
         */
        if (notifyClose) {
            final ASN1BufferWriter asn1Writer = ASN1BufferWriter.getWriter();
            try {
                final ASN1BufferWriter asn1Writer = ASN1BufferWriter.getWriter();
                try {
                    ldapWriter
                            .unbindRequest(asn1Writer, nextMsgID.getAndIncrement(), unbindRequest);
                    connection.write(asn1Writer.getBuffer(), null);
                } finally {
                    asn1Writer.recycle();
                }
            } catch (final IOException e) {
                // Underlying channel prob blown up. Just ignore.
                ldapWriter.unbindRequest(asn1Writer, nextMsgID.getAndIncrement(), unbindRequest);
                connection.write(asn1Writer.getBuffer(), null);
            } catch (final Exception ignore) {
                /*
                 * Underlying channel probably blown up. Ignore all errors,
                 * including possibly runtime exceptions (see OPENDJ-672).
                 */
            } finally {
                asn1Writer.recycle();
            }
            factory.getTimeoutChecker().removeConnection(this);
            connection.closeSilently();
            factory.releaseTransportAndTimeoutChecker();
        }
        factory.getTimeoutChecker().removeConnection(this);
        connection.closeSilently();
        factory.releaseTransportAndTimeoutChecker();

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

 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();
    }
}