[geode] branch develop updated: GEODE-6011: Prevent synchronization using strings and boolean (#2807)

[jb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new fcc61b6  GEODE-6011: Prevent synchronization using strings and boolean (#2807)
fcc61b6 is described below

commit fcc61b647f147fba409cd4c73de8271b7bc78c2f
Author: Nabarun Nag <na...@users.noreply.github.com>
AuthorDate: Fri Jan 4 23:09:30 2019 -0800

    GEODE-6011: Prevent synchronization using strings and boolean (#2807)
    
    * Locking is now achieved using reentrant locks as they are more fair than synchronization.
---
 .../geode/modules/util/BootstrappingFunction.java      |  7 ++++++-
 .../org/apache/geode/internal/net/SocketCloser.java    | 18 ++++++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java
index bc399e3..4cc9c09 100644
--- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java
+++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/BootstrappingFunction.java
@@ -21,6 +21,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.geode.DataSerializable;
 import org.apache.geode.cache.Cache;
@@ -42,6 +43,7 @@ public class BootstrappingFunction implements Function, MembershipListener, Data
   private static final long serialVersionUID = 1856043174458190605L;
 
   public static final String ID = "bootstrapping-function";
+  private static final ReentrantLock registerFunctionLock = new ReentrantLock();
 
   private static final int TIME_TO_WAIT_FOR_CACHE =
       Integer.getInteger("gemfiremodules.timeToWaitForCache", 30000);
@@ -105,7 +107,8 @@ public class BootstrappingFunction implements Function, MembershipListener, Data
   private void registerFunctions() {
     // Synchronize so that these functions aren't registered twice. The
     // constructor for the CreateRegionFunction creates a meta region.
-    synchronized (ID) {
+    registerFunctionLock.lock();
+    try {
       // Register the create region function if it is not already registered
       if (!FunctionService.isRegistered(CreateRegionFunction.ID)) {
         FunctionService.registerFunction(new CreateRegionFunction());
@@ -125,6 +128,8 @@ public class BootstrappingFunction implements Function, MembershipListener, Data
       if (!FunctionService.isRegistered(RegionSizeFunction.ID)) {
         FunctionService.registerFunction(new RegionSizeFunction());
       }
+    } finally {
+      registerFunctionLock.unlock();
     }
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java
index 53815ec..d33caba 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java
@@ -22,6 +22,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.geode.SystemFailure;
 import org.apache.geode.internal.logging.LoggingExecutors;
@@ -68,6 +69,13 @@ public class SocketCloser {
   private final int asyncClosePoolMaxThreads;
   private final long asyncCloseWaitTime;
   private final TimeUnit asyncCloseWaitUnits;
+  /**
+   * Protect access to closed synchronizing with closedLock
+   */
+  private final ReentrantLock closedLock = new ReentrantLock();
+  /**
+   * Protect access to closed using synchronizing with closedLock
+   */
   private boolean closed;
 
   public SocketCloser() {
@@ -132,12 +140,15 @@ public class SocketCloser {
    * called then the asyncClose will be done synchronously.
    */
   public void close() {
-    synchronized (this) {
+    closedLock.lock();
+    try {
       if (!this.closed) {
         this.closed = true;
       } else {
         return;
       }
+    } finally {
+      closedLock.unlock();
     }
     for (ExecutorService executorService : asyncCloseExecutors.values()) {
       executorService.shutdown();
@@ -167,7 +178,8 @@ public class SocketCloser {
     boolean doItInline = false;
     try {
       Future submittedTask = null;
-      synchronized (this) {
+      closedLock.lock();
+      try {
         if (closed) {
           // this SocketCloser has been closed so do a synchronous, inline, close
           doItInline = true;
@@ -186,6 +198,8 @@ public class SocketCloser {
             }
           });
         }
+      } finally {
+        closedLock.unlock();
       }
       if (submittedTask != null) {
         waitForFutureTaskWithTimeout(submittedTask);