Mailing-List: contact commits-help@geode.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: dschneider@apache.org
To: commits@geode.incubator.apache.org
Date: Fri, 29 Jan 2016 01:16:48 -0000
Message-Id: <027255a7007f4dee8d8b1ce4398e36ad@git.apache.org>
In-Reply-To: <7953555325cd412f9d6803766f987267@git.apache.org>
References: <7953555325cd412f9d6803766f987267@git.apache.org>
Subject: [31/52] [abbrv] incubator-geode git commit: GEODE-857: Fixing
 synchronization in SystemFailure.stopWatchDog

GEODE-857: Fixing synchronization in SystemFailure.stopWatchDog

Don't join the watchDog thread or the proctor thread while holding the
failureSync lock.

Adding a unit test for SystemFailure.stopThreads.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/30d2d64d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/30d2d64d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/30d2d64d

Branch: refs/heads/feature/GEODE-831
Commit: 30d2d64d02c50df54d98aede41cdbbe56569dc43
Parents: 45d5eda
Author: Dan Smith <upthewaterspout@apache.org>
Authored: Mon Jan 25 16:42:25 2016 -0800
Committer: Dan Smith <upthewaterspout@apache.org>
Committed: Tue Jan 26 17:01:37 2016 -0800

----------------------------------------------------------------------
 .../com/gemstone/gemfire/SystemFailure.java     | 49 ++++++++++------
 .../gemfire/SystemFailureJUnitTest.java         | 60 ++++++++++++++++++++
 2 files changed, 93 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/30d2d64d/gemfire-core/src/main/java/com/gemstone/gemfire/SystemFailure.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/SystemFailure.java b/gemfire-core/src/main/java/com/gemstone/gemfire/SystemFailure.java
index 494f5f7..1775063 100644
--- a/gemfire-core/src/main/java/com/gemstone/gemfire/SystemFailure.java
+++ b/gemfire-core/src/main/java/com/gemstone/gemfire/SystemFailure.java
@@ -192,6 +192,11 @@ import com.gemstone.gemfire.internal.i18n.LocalizedStrings;
 public final class SystemFailure {
 
   /**
+   * Time to wait during stopWatchdog and stopProctor. Not final
+   * for tests
+   */
+  static int SHUTDOWN_WAIT = 1000;
+  /**
    * Preallocated error messages\
    * LocalizedStrings may use memory (in the form of an iterator)
    * so we must get the translated messages in advance.
@@ -384,23 +389,26 @@ public final class SystemFailure {
   }
   
   private static void stopWatchDog() {
+    Thread watchDogSnapshot = null;
     synchronized (failureSync) {
       stopping = true;
       if (watchDog != null && watchDog.isAlive()) {
         failureSync.notifyAll();
+        watchDogSnapshot = watchDog;
+      }
+    }
+    if(watchDogSnapshot != null) {
+      try {
+        watchDogSnapshot.join(100);
+      } catch (InterruptedException ignore) {
+      }
+      if (watchDogSnapshot.isAlive()) {
+        watchDogSnapshot.interrupt();
         try {
-          watchDog.join(100);
+          watchDogSnapshot.join(SHUTDOWN_WAIT);
         } catch (InterruptedException ignore) {
         }
-        if (watchDog.isAlive()) {
-          watchDog.interrupt();
-          try {
-            watchDog.join(1000);
-          } catch (InterruptedException ignore) {
-          }
-        }
       }
-      watchDog = null;
     }
   }
   
@@ -634,16 +642,17 @@ public final class SystemFailure {
   }
   
   private static void stopProctor() {
+    Thread proctorSnapshot = null;
     synchronized (failureSync) {
       stopping = true;
-      if (proctor != null && proctor.isAlive()) {
-        proctor.interrupt();
-        try {
-          proctor.join(1000);
-        } catch (InterruptedException ignore) {
-        }
+      proctorSnapshot = proctor;
+    }
+    if (proctorSnapshot != null && proctorSnapshot.isAlive()) {
+      proctorSnapshot.interrupt();
+      try {
+        proctorSnapshot.join(SHUTDOWN_WAIT);
+      } catch (InterruptedException ignore) {
       }
-      proctor = null;
     }
   }
   
@@ -1219,4 +1228,12 @@ public final class SystemFailure {
     stopProctor();
     stopWatchDog();
   }
+  
+  static Thread getWatchDogForTest() {
+    return watchDog;
+  }
+  
+  static Thread getProctorForTest() {
+    return proctor;
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/30d2d64d/gemfire-core/src/test/java/com/gemstone/gemfire/SystemFailureJUnitTest.java
----------------------------------------------------------------------
diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/SystemFailureJUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/SystemFailureJUnitTest.java
new file mode 100644
index 0000000..e33eba4
--- /dev/null
+++ b/gemfire-core/src/test/java/com/gemstone/gemfire/SystemFailureJUnitTest.java
@@ -0,0 +1,60 @@
+/*
+ * 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 com.gemstone.gemfire;
+
+import static org.junit.Assert.*;
+
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import com.gemstone.gemfire.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class SystemFailureJUnitTest {
+
+  private static final int LONG_WAIT = 30000;
+  private int oldWaitTime;
+  @Before
+  public void setWaitTime() {
+    oldWaitTime = SystemFailure.SHUTDOWN_WAIT;
+    SystemFailure.SHUTDOWN_WAIT = LONG_WAIT;
+  }
+  
+  @After
+  public void restoreWaitTime() {
+    SystemFailure.SHUTDOWN_WAIT = oldWaitTime;
+  }
+  @Test
+  public void testStopThreads() {
+    SystemFailure.startThreads();
+    long start = System.nanoTime();
+    Thread watchDog = SystemFailure.getWatchDogForTest();
+    Thread proctor= SystemFailure.getProctorForTest();
+    assertTrue(watchDog.isAlive());
+    assertTrue(proctor.isAlive());
+    SystemFailure.stopThreads();
+    long elapsed = System.nanoTime() - start;
+    assertTrue("Waited too long to shutdown: " + elapsed, elapsed < TimeUnit.MILLISECONDS.toNanos(LONG_WAIT));
+    assertFalse(watchDog.isAlive());
+    assertFalse(proctor.isAlive());
+  }
+
+}