Skip to content

Commit

Permalink
GEODE-8473: Hang in ReplyProcessor21 when forced-disconnect does not …
Browse files Browse the repository at this point in the history
…establish a cancellation cause (apache#5491)

Ensure that the cache is informed of a forced-disconnect in the
DisconnectThread.  This is a follow-on commit to GEODE-8467, which
ensured that the DisconnectThread is launched in the presence of cache
XML generation failure.  This commit adds a try/catch in
GMSMembership.uncleanShutdown() to ensure that the up-stream
ClusterDistributionManager is informed of the failure so it can set the
"rootCause" in its CancelCriterion.  ReplyProcessor21 and other objects
that poll for this "rootCause" will then be released from waiting for
responses to messages sent to other members of the cluster.
  • Loading branch information
bschuchardt authored Sep 16, 2020
1 parent 1d629e1 commit c48c0c3
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import static org.apache.geode.internal.serialization.DataSerializableFixedID.HIGH_PRIORITY_ACKED_MESSAGE;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
Expand All @@ -45,6 +47,7 @@

import org.apache.geode.distributed.internal.membership.api.Authenticator;
import org.apache.geode.distributed.internal.membership.api.LifecycleListener;
import org.apache.geode.distributed.internal.membership.api.MemberDisconnectedException;
import org.apache.geode.distributed.internal.membership.api.MemberIdentifier;
import org.apache.geode.distributed.internal.membership.api.MemberShunnedException;
import org.apache.geode.distributed.internal.membership.api.MemberStartupException;
Expand Down Expand Up @@ -396,6 +399,18 @@ public void testMulticastAllowedWithNewVersionViewMember() {
assertThat(manager.getGMSManager().isMulticastAllowed()).isTrue();
}

@Test
public void membershipInvokesUpstreamListenerDuringForcedDisconnect() {
// have an exception interrupt the shutdown process and ensure that a thread is
// launched to inform the cache of shutdown
IllegalStateException expectedException = new IllegalStateException();
doThrow(expectedException).when(services).emergencyClose();
assertThatThrownBy(() -> manager.uncleanShutdown("For testing",
new MemberDisconnectedException("For Testing")))
.isEqualTo(expectedException);
verify(listener).membershipFailure(isA(String.class), isA(Throwable.class));
}

private MemberIdentifier createSurpriseMember(Version version) {
MemberIdentifier surpriseMember = createMemberID(DEFAULT_PORT + 5);
surpriseMember.setVmViewId(3);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1267,25 +1267,27 @@ public void shutdown() {
public void uncleanShutdown(String reason, final Exception e) {
inhibitForcedDisconnectLogging(false);

if (services.getShutdownCause() == null) {
services.setShutdownCause(e);
}

if (cleanupTimer != null && !cleanupTimer.isShutdown()) {
cleanupTimer.shutdownNow();
}
try {
if (services.getShutdownCause() == null) {
services.setShutdownCause(e);
}

lifecycleListener.disconnect(e);
if (cleanupTimer != null && !cleanupTimer.isShutdown()) {
cleanupTimer.shutdownNow();
}

// first shut down communication so we don't do any more harm to other
// members
services.emergencyClose();
lifecycleListener.disconnect(e);

if (e != null) {
try {
listener.membershipFailure(reason, e);
} catch (RuntimeException re) {
logger.warn("Exception caught while shutting down", re);
// first shut down communication so we don't do any more harm to other
// members
services.emergencyClose();
} finally {
if (e != null) {
try {
listener.membershipFailure(reason, e);
} catch (RuntimeException re) {
logger.warn("Exception caught while shutting down", re);
}
}
}
}
Expand Down

0 comments on commit c48c0c3

Please sign in to comment.