Merge pull request apache#3882 from Nordix/feature/GEODE-7036

GEODE-7036: Fix for NPE caused by ex-coordinator joining quorum

geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/RestartOfMemberDistributedTest.java

@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.distributed.internal;
+
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.ForcedDisconnectException;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+
+
+public class RestartOfMemberDistributedTest {
+  public List<MemberVM> locators = new ArrayList<>();
+  public List<MemberVM> servers = new ArrayList<>();
+
+
+
+  @Rule
+  public ClusterStartupRule clusterStartupRule = new ClusterStartupRule();
+
+  @Before
+  public void before() {
+    Properties properties = new Properties();
+
+    locators.add(clusterStartupRule.startLocatorVM(0, properties));
+    servers.add(clusterStartupRule.startServerVM(1, properties, locators.get(0).getPort()));
+    locators.add(clusterStartupRule.startLocatorVM(2, properties, locators.get(0).getPort()));
+    servers.add(clusterStartupRule.startServerVM(3, properties, locators.get(0).getPort()));
+  }
+
+  @After
+  public void after() {
+    servers.clear();
+    locators.clear();
+  }
+
+  @Test
+  public void exCoordinatorJoiningQuorumDoesNotThrowNullPointerException() {
+    IgnoredException exp1 =
+        IgnoredException.addIgnoredException(ForcedDisconnectException.class.getName());
+    IgnoredException exp2 =
+        IgnoredException.addIgnoredException("Possible loss of quorum due to the loss");
+    IgnoredException exp3 =
+        IgnoredException.addIgnoredException("Received invalid result from");
+    try {
+      int locator2port = locators.get(1).getPort();
+      Properties properties = new Properties();
+      int locator0port = locators.get(0).getPort();
+      clusterStartupRule.crashVM(1);
+      clusterStartupRule.crashVM(0);
+      await().until(() -> {
+        clusterStartupRule.startLocatorVM(0, locator0port, properties, locator2port);
+        return true;
+      });
+      clusterStartupRule.startServerVM(1, properties, locator2port);
+      locators.get(1).waitTilFullyReconnected();
+    } finally {
+      exp1.remove();
+      exp2.remove();
+      exp3.remove();
+    }
+  }
+
+}

geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java

@@ -1111,6 +1111,11 @@ private boolean attemptReconnect() throws InterruptedException, IOException {
       InternalDistributedSystem newSystem =
           (InternalDistributedSystem) system.getReconnectedSystem();
       if (newSystem != null) {
+        boolean noprevlocator = false;
+        if (!hasLocator()) {
+          setLocator(this);
+          noprevlocator = true;
+        }
         if (!tcpServerStarted) {
           if (locatorListener != null) {
             locatorListener.clearLocatorInfo();
@@ -1122,10 +1127,12 @@ private boolean attemptReconnect() throws InterruptedException, IOException {
           restartWithSystem(newSystem, GemFireCacheImpl.getInstance());
         } catch (CancelException e) {
           stoppedForReconnect = true;
+          if (noprevlocator) {
+            removeLocator(this);
+          }
           return false;
         }
 
-        setLocator(this);
         restarted = true;
       }
     }

geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClusterStartupRule.java

@@ -211,16 +211,21 @@ public MemberVM startLocatorVM(int index, Properties properties, int... locatorP
         x -> x.withProperties(properties).withConnectionToLocator(locatorPort));
   }
 
+  public MemberVM startLocatorVM(int index, int port, Properties properties, int... locatorPort) {
+    return startLocatorVM(index, port, VersionManager.CURRENT_VERSION,
+        x -> x.withProperties(properties).withConnectionToLocator(locatorPort));
+  }
+
   public MemberVM startLocatorVM(int index, String version) {
-    return startLocatorVM(index, version, x -> x);
+    return startLocatorVM(index, 0, version, x -> x);
   }
 
   public MemberVM startLocatorVM(int index,
       SerializableFunction<LocatorStarterRule> ruleOperator) {
-    return startLocatorVM(index, VersionManager.CURRENT_VERSION, ruleOperator);
+    return startLocatorVM(index, 0, VersionManager.CURRENT_VERSION, ruleOperator);
   }
 
-  public MemberVM startLocatorVM(int index, String version,
+  public MemberVM startLocatorVM(int index, int port, String version,
       SerializableFunction<LocatorStarterRule> ruleOperator) {
     final String defaultName = "locator-" + index;
     VM locatorVM = getVM(index, version);
@@ -233,6 +238,9 @@ public MemberVM startLocatorVM(int index, String version,
       ruleOperator.apply(locatorStarter);
       locatorStarter.withName(defaultName);
       locatorStarter.withAutoStart();
+      if (port != 0) {
+        locatorStarter.withPort(port);
+      }
       locatorStarter.before();
       return locatorStarter;
     });