You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2019/06/10 15:38:21 UTC
[geode] branch develop updated: GEODE-6829: Fixes synchronization
in AttributesFactory (#3670)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao 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 7693689 GEODE-6829: Fixes synchronization in AttributesFactory (#3670)
7693689 is described below
commit 769368999c24ec469b6ca8d323cd8de960b069fb
Author: Joris Melchior <jo...@gmail.com>
AuthorDate: Mon Jun 10 11:38:08 2019 -0400
GEODE-6829: Fixes synchronization in AttributesFactory (#3670)
Co-authored-by: Joris Melchior <jo...@gmail.com>
Co-authored-by: Peter Tran <pt...@gmail.com>
---
.../org/apache/geode/cache/AttributesFactory.java | 76 +++++++++-------------
.../geode/cache/AttributesFactoryJUnitTest.java | 33 ++++++++++
2 files changed, 65 insertions(+), 44 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java b/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
index c000227..c9ff9e2 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java
@@ -1688,33 +1688,27 @@ public class AttributesFactory<K, V> {
@Override
@SuppressWarnings("unchecked")
public CacheListener<K, V>[] getCacheListeners() {
- ArrayList<CacheListener<K, V>> listeners = this.cacheListeners;
- if (listeners == null) {
- return (CacheListener<K, V>[]) EMPTY_LISTENERS;
- } else {
- synchronized (listeners) {
- if (listeners.size() == 0) {
- return (CacheListener<K, V>[]) EMPTY_LISTENERS;
- } else {
- CacheListener<K, V>[] result = new CacheListener[listeners.size()];
- listeners.toArray(result);
- return result;
- }
+ synchronized (this) {
+ if (this.cacheListeners == null || this.cacheListeners.isEmpty()) {
+ return (CacheListener<K, V>[]) EMPTY_LISTENERS;
+ } else {
+ CacheListener<K, V>[] result = new CacheListener[this.cacheListeners.size()];
+ this.cacheListeners.toArray(result);
+ return result;
}
}
}
@Override
public CacheListener<K, V> getCacheListener() {
- ArrayList<CacheListener<K, V>> listeners = this.cacheListeners;
- if (listeners == null) {
- return null;
- }
- synchronized (listeners) {
- if (listeners.size() == 0) {
+ synchronized (this) {
+ if (this.cacheListeners == null) {
return null;
}
- if (listeners.size() == 1) {
+ if (this.cacheListeners.size() == 0) {
+ return null;
+ }
+ if (this.cacheListeners.size() == 1) {
return this.cacheListeners.get(0);
}
}
@@ -1723,27 +1717,23 @@ public class AttributesFactory<K, V> {
}
protected void addCacheListener(CacheListener<K, V> aListener) {
- ArrayList<CacheListener<K, V>> listeners = this.cacheListeners;
- if (listeners == null) {
- ArrayList<CacheListener<K, V>> al = new ArrayList<CacheListener<K, V>>(1);
- al.add(aListener);
- this.cacheListeners = al;
- } else {
- synchronized (listeners) {
- listeners.add(aListener);
+ synchronized (this) {
+ if (this.cacheListeners == null) {
+ this.cacheListeners = new ArrayList<CacheListener<K, V>>(1);
+ this.cacheListeners.add(aListener);
+ } else {
+ this.cacheListeners.add(aListener);
}
+ setHasCacheListeners(true);
}
- setHasCacheListeners(true);
}
public void addGatewaySenderId(String gatewaySenderId) {
- if (this.gatewaySenderIds == null) {
- this.gatewaySenderIds = new CopyOnWriteArraySet<String>();
- this.gatewaySenderIds.add(gatewaySenderId);
- } else {
- synchronized (this.gatewaySenderIds) { // TODO: revisit this
- // synchronization : added as per
- // above code
+ synchronized (this) {
+ if (this.gatewaySenderIds == null) {
+ this.gatewaySenderIds = new CopyOnWriteArraySet<String>();
+ this.gatewaySenderIds.add(gatewaySenderId);
+ } else {
if (this.gatewaySenderIds.contains(gatewaySenderId)) {
throw new IllegalArgumentException(
String.format("gateway-sender-id %s is already added",
@@ -1751,18 +1741,16 @@ public class AttributesFactory<K, V> {
}
this.gatewaySenderIds.add(gatewaySenderId);
}
+ setHasGatewaySenderIds(true);
}
- setHasGatewaySenderIds(true);
}
public void addAsyncEventQueueId(String asyncEventQueueId) {
- if (this.asyncEventQueueIds == null) {
- this.asyncEventQueueIds = new CopyOnWriteArraySet<String>();
- this.asyncEventQueueIds.add(asyncEventQueueId);
- } else {
- synchronized (this.asyncEventQueueIds) { // TODO: revisit this
- // synchronization : added as per
- // above code
+ synchronized (this) {
+ if (this.asyncEventQueueIds == null) {
+ this.asyncEventQueueIds = new CopyOnWriteArraySet<String>();
+ this.asyncEventQueueIds.add(asyncEventQueueId);
+ } else {
if (this.asyncEventQueueIds.contains(asyncEventQueueId)) {
throw new IllegalArgumentException(
String.format("async-event-queue-id %s is already added",
@@ -1770,8 +1758,8 @@ public class AttributesFactory<K, V> {
}
this.asyncEventQueueIds.add(asyncEventQueueId);
}
+ setHasAsyncEventListeners(true);
}
- setHasAsyncEventListeners(true);
}
@Override
diff --git a/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java
index e6d0270..95d91a8 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/AttributesFactoryJUnitTest.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.fail;
import java.io.File;
import java.util.Arrays;
+import java.util.Set;
import org.junit.Test;
@@ -421,4 +422,36 @@ public class AttributesFactoryJUnitTest {
private static class MyCacheListener extends CacheListenerAdapter {
// empty impl
}
+
+ @Test
+ public void addsGatewaySenderId() {
+ AttributesFactory factory = new AttributesFactory();
+ factory.addGatewaySenderId("someSenderId");
+ RegionAttributes regionAttributes = factory.create();
+ Set gatewaySenderIds = regionAttributes.getGatewaySenderIds();
+
+ assertTrue(gatewaySenderIds.contains("someSenderId"));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void addingNullGatewaySenderIdThrowsException() {
+ AttributesFactory factory = new AttributesFactory();
+ factory.addGatewaySenderId(null);
+ }
+
+ @Test
+ public void addsAsyncEventQueueId() {
+ AttributesFactory factory = new AttributesFactory();
+ factory.addAsyncEventQueueId("someId");
+ RegionAttributes regionAttributes = factory.create();
+ Set asyncEventQueueIds = regionAttributes.getAsyncEventQueueIds();
+
+ assertTrue(asyncEventQueueIds.contains("someId"));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void addingNullAsyncEventQueueIdThrowsException() {
+ AttributesFactory factory = new AttributesFactory();
+ factory.addAsyncEventQueueId(null);
+ }
}