You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2016/12/19 17:51:30 UTC
[4/5] geode git commit: GEODE-2215 NPE in ViewCreator thread setting
public keys into a NetView
GEODE-2215 NPE in ViewCreator thread setting public keys into a NetView
NetView creates a ConcurrentHashmap to hold the public keys when it's
constructed but it had some methods that were replacing it with a
Hashmap. I made the field final and also added checks to avoid putting
a null key or value into the map.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/5dfce1bd
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/5dfce1bd
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/5dfce1bd
Branch: refs/heads/develop
Commit: 5dfce1bd6fe625042abf243dec60098a9394ef27
Parents: f68c416
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Mon Dec 19 09:07:52 2016 -0800
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Mon Dec 19 09:50:51 2016 -0800
----------------------------------------------------------------------
.../internal/membership/NetView.java | 50 +-
.../internal/membership/NetViewJUnitTest.java | 16 +
.../sanctionedDataSerializables.txt | 1064 +++++++++---------
3 files changed, 581 insertions(+), 549 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/5dfce1bd/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
index ca62e20..26b0327 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/NetView.java
@@ -14,21 +14,29 @@
*/
package org.apache.geode.distributed.internal.membership;
-import java.io.DataInput;
-import java.io.DataOutput;
-import java.io.IOException;
-import java.util.stream.*;
-import java.util.*;
-import java.util.concurrent.ConcurrentHashMap;
-
-import org.apache.logging.log4j.Logger;
-
import org.apache.geode.DataSerializer;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.internal.DistributionManager;
import org.apache.geode.internal.DataSerializableFixedID;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.Version;
+import org.apache.logging.log4j.Logger;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
/**
* The NetView class represents a membership view. Note that this class is not synchronized, so take
@@ -41,7 +49,7 @@ public class NetView implements DataSerializableFixedID {
private int viewId;
private List<InternalDistributedMember> members;
// TODO this should be a List
- private Map<InternalDistributedMember, Object> publicKeys = new ConcurrentHashMap<>();
+ private final Map<InternalDistributedMember, Object> publicKeys = new ConcurrentHashMap<>();
private int[] failureDetectionPorts = new int[10];
private Set<InternalDistributedMember> shutdownMembers;
private Set<InternalDistributedMember> crashedMembers;
@@ -113,7 +121,7 @@ public class NetView implements DataSerializableFixedID {
other.failureDetectionPorts.length);
this.shutdownMembers = new HashSet<>(other.shutdownMembers);
this.crashedMembers = new HashSet<>(other.crashedMembers);
- this.publicKeys = new HashMap<>(other.publicKeys);
+ this.publicKeys.putAll(other.publicKeys);
}
public NetView(InternalDistributedMember creator, int viewId,
@@ -146,14 +154,19 @@ public class NetView implements DataSerializableFixedID {
}
public void setPublicKey(InternalDistributedMember mbr, Object key) {
- publicKeys.put(mbr, key);
+ if (mbr != null && key != null) {
+ publicKeys.put(mbr, key);
+ }
}
public void setPublicKeys(NetView otherView) {
- this.publicKeys.putAll(otherView.publicKeys);
+ if (otherView.publicKeys != null) {
+ this.publicKeys.putAll(otherView.publicKeys);
+ }
}
+
public int[] getFailureDetectionPorts() {
return this.failureDetectionPorts;
}
@@ -558,10 +571,10 @@ public class NetView implements DataSerializableFixedID {
if (other == this) {
return true;
}
- if (!(other instanceof NetView)) {
- return false;
+ if (other instanceof NetView) {
+ return this.members.equals(((NetView) other).getMembers());
}
- return this.members.equals(((NetView) other).getMembers());
+ return false;
}
@Override
@@ -591,7 +604,10 @@ public class NetView implements DataSerializableFixedID {
shutdownMembers = InternalDataSerializer.readHashSet(in);
crashedMembers = InternalDataSerializer.readHashSet(in);
failureDetectionPorts = DataSerializer.readIntArray(in);
- publicKeys = DataSerializer.readHashMap(in);
+ Map pubkeys = DataSerializer.readHashMap(in);
+ if (pubkeys != null) {
+ publicKeys.putAll(pubkeys);
+ }
}
/** this will deserialize as an ArrayList */
http://git-wip-us.apache.org/repos/asf/geode/blob/5dfce1bd/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/NetViewJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/NetViewJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/NetViewJUnitTest.java
index 715da47..fef77de 100755
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/NetViewJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/NetViewJUnitTest.java
@@ -193,6 +193,22 @@ public class NetViewJUnitTest {
assertEquals(100, view.getNewMembers(copy).size());
}
+ @Test
+ public void testNullPublicKeysNotRetained() throws Exception {
+ NetView view = new NetView(members.get(0), 2, new ArrayList<>(members));
+ setFailureDetectionPorts(view);
+
+ NetView newView = new NetView(view, 3);
+ for (InternalDistributedMember member : view.getMembers()) {
+ view.setPublicKey(member, null);
+ }
+ newView.setPublicKeys(view);
+ for (InternalDistributedMember member : view.getMembers()) {
+ assertNull(newView.getPublicKey(member));
+ assertNull(view.getPublicKey(member));
+ }
+ }
+
/**
* Test that failed weight calculations are correctly performed. See bug #47342
*/