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

nnag 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 5410f93  GEODE-5982: Synchronized access to CacheLoader and 
CacheWriter (#2776)
5410f93 is described below

commit 5410f9390432d13e8f5a4e054c4145dac9e755c1
Author: Nabarun Nag <nabarun...@users.noreply.github.com>
AuthorDate: Thu Nov 8 09:49:05 2018 -0800

    GEODE-5982: Synchronized access to CacheLoader and CacheWriter (#2776)
    
    * Using read-write locks to synchronize the getters and setters for 
CacheLoader and CacheWriter.
---
 .../geode/internal/cache/AbstractRegion.java       | 95 ++++++++++++++++------
 1 file changed, 72 insertions(+), 23 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java 
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
index bb99ce0..b504b33 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
@@ -28,6 +28,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.logging.log4j.Logger;
 
@@ -100,7 +101,8 @@ public abstract class AbstractRegion implements 
InternalRegion, AttributesMutato
     DataSerializableFixedID, Extensible<Region<?, ?>>, EvictableRegion {
 
   private static final Logger logger = LogService.getLogger();
-
+  private final ReentrantReadWriteLock readWriteLockForCacheLoader = new 
ReentrantReadWriteLock();
+  private final ReentrantReadWriteLock readWriteLockForCacheWriter = new 
ReentrantReadWriteLock();
   /**
    * Identifies the static order in which this region was created in relation 
to other regions or
    * other instances of this region during the life of this JVM.
@@ -121,8 +123,14 @@ public abstract class AbstractRegion implements 
InternalRegion, AttributesMutato
    */
   private volatile CacheListener[] cacheListeners;
 
+  /**
+   * All access to cacheLoader must be protected by readWriteLockForCacheLoader
+   */
   private volatile CacheLoader cacheLoader;
 
+  /**
+   * All access to cacheWriter must be protected by readWriteLockForCacheWriter
+   */
   private volatile CacheWriter cacheWriter;
 
   protected int entryIdleTimeout;
@@ -519,12 +527,22 @@ public abstract class AbstractRegion implements 
InternalRegion, AttributesMutato
 
   @Override
   public CacheLoader getCacheLoader() {
-    return this.cacheLoader;
+    readWriteLockForCacheLoader.readLock().lock();
+    try {
+      return this.cacheLoader;
+    } finally {
+      readWriteLockForCacheLoader.readLock().unlock();
+    }
   }
 
   @Override
   public CacheWriter getCacheWriter() {
-    return this.cacheWriter;
+    readWriteLockForCacheWriter.readLock().lock();
+    try {
+      return this.cacheWriter;
+    } finally {
+      readWriteLockForCacheWriter.readLock().unlock();
+    }
   }
 
   /**
@@ -1140,29 +1158,49 @@ public abstract class AbstractRegion implements 
InternalRegion, AttributesMutato
   }
 
   @Override
-  public synchronized CacheLoader setCacheLoader(CacheLoader cacheLoader) {
-    checkReadiness();
-    CacheLoader oldLoader = this.cacheLoader;
-    assignCacheLoader(cacheLoader);
-    cacheLoaderChanged(oldLoader);
-    return oldLoader;
+  public CacheLoader setCacheLoader(CacheLoader cacheLoader) {
+    readWriteLockForCacheLoader.writeLock().lock();
+    try {
+      checkReadiness();
+      CacheLoader oldLoader = this.cacheLoader;
+      this.cacheLoader = cacheLoader;
+      cacheLoaderChanged(oldLoader);
+      return oldLoader;
+    } finally {
+      readWriteLockForCacheLoader.writeLock().unlock();
+    }
   }
 
-  private synchronized void assignCacheLoader(CacheLoader cl) {
-    this.cacheLoader = cl;
+  private void assignCacheLoader(CacheLoader cl) {
+    readWriteLockForCacheLoader.writeLock().lock();
+    try {
+      this.cacheLoader = cl;
+    } finally {
+      readWriteLockForCacheLoader.writeLock().unlock();
+    }
   }
 
   @Override
-  public synchronized CacheWriter setCacheWriter(CacheWriter cacheWriter) {
-    checkReadiness();
-    CacheWriter oldWriter = this.cacheWriter;
-    assignCacheWriter(cacheWriter);
-    cacheWriterChanged(oldWriter);
-    return oldWriter;
+  public CacheWriter setCacheWriter(CacheWriter cacheWriter) {
+    readWriteLockForCacheWriter.writeLock().lock();
+    try {
+      checkReadiness();
+      CacheWriter oldWriter = this.cacheWriter;
+      this.cacheWriter = cacheWriter;
+      cacheWriterChanged(oldWriter);
+      return oldWriter;
+    } finally {
+      readWriteLockForCacheWriter.writeLock().unlock();
+    }
   }
 
-  private synchronized void assignCacheWriter(CacheWriter cacheWriter) {
-    this.cacheWriter = cacheWriter;
+  private void assignCacheWriter(CacheWriter cacheWriter) {
+    readWriteLockForCacheWriter.writeLock().lock();
+    try {
+      this.cacheWriter = cacheWriter;
+    } finally {
+      readWriteLockForCacheWriter.writeLock().unlock();
+    }
   }
 
   void checkEntryTimeoutAction(String mode, ExpirationAction ea) {
@@ -1503,9 +1541,15 @@ public abstract class AbstractRegion implements 
InternalRegion, AttributesMutato
   }
 
   protected void cacheLoaderChanged(CacheLoader oldLoader) {
-    if (this.cacheLoader != oldLoader) {
-      closeCacheCallback(oldLoader);
+    readWriteLockForCacheLoader.readLock().lock();
+    try {
+      if (this.cacheLoader != oldLoader) {
+        closeCacheCallback(oldLoader);
+      }
+    } finally {
+      readWriteLockForCacheLoader.readLock().unlock();
     }
+
   }
 
   /**
@@ -1519,8 +1563,13 @@ public abstract class AbstractRegion implements 
InternalRegion, AttributesMutato
   }
 
   protected void cacheWriterChanged(CacheWriter oldWriter) {
-    if (this.cacheWriter != oldWriter) {
-      closeCacheCallback(oldWriter);
+    readWriteLockForCacheWriter.readLock().lock();
+    try {
+      if (this.cacheWriter != oldWriter) {
+        closeCacheCallback(oldWriter);
+      }
+    } finally {
+      readWriteLockForCacheWriter.readLock().unlock();
     }
   }
 

Reply via email to