You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by la...@apache.org on 2018/05/01 23:39:08 UTC
[geode] 01/01: GEODE-5166: NPE thrown while processing InitialImage
of subscription region
This is an automated email from the ASF dual-hosted git repository.
ladyvader pushed a commit to branch feature/GEODE-5166
in repository https://gitbox.apache.org/repos/asf/geode.git
commit ed3d10c011417e1c7a7056d2716a8244ff235c5a
Author: Lynn Hughes-Godfrey <lh...@pivotal.io>
AuthorDate: Tue May 1 16:35:28 2018 -0700
GEODE-5166: NPE thrown while processing InitialImage of subscription region
* Fix NPE in updateHAEventWrapper
* Clean up code (renaming variables) in putEventInHARegion
* Removing old/commented out code
---
.../geode/internal/cache/ha/HARegionQueue.java | 114 +++++++--------------
1 file changed, 38 insertions(+), 76 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
index 9e0b38c..a49adfb 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
@@ -2107,40 +2107,23 @@ public class HARegionQueue implements RegionQueue {
continue;
}
synchronized (entryHaEventWrapper) {
- if (haContainer.getKey(entryHaEventWrapper) != null) {
+ if ((HAEventWrapper) haContainer.getKey(entryHaEventWrapper) != null) {
entryHaEventWrapper.incAndGetReferenceCount();
- // If the input and entry HAEventWrappers are not the same (which is the normal
- // case), add the CQs and interest list from the input to the entry and create a new
- // value from the entry.
- if (entryHaEventWrapper != inputHaEventWrapper) { // See GEODE-4957
- addClientCQsAndInterestList(entryMessage, inputHaEventWrapper, haContainer,
- regionName);
- inputHaEventWrapper.setClientUpdateMessage(null);
- newValueCd =
- new VMCachedDeserializable(entryHaEventWrapper, newValueCd.getSizeInBytes());
- }
- } else {
- entryHaEventWrapper = null;
- }
- }
- } else { // putIfAbsent successful
- entryHaEventWrapper = (HAEventWrapper) haContainer.getKey(inputHaEventWrapper);
- synchronized (entryHaEventWrapper) {
- entryHaEventWrapper.incAndGetReferenceCount();
- entryHaEventWrapper.setHAContainer(haContainer);
- // If the input and entry HAEventWrappers are not the same (which is not the normal
- // case), get the entry message, add the CQs and interest list from the input to the
- // entry and create a new value from the entry.
- if (entryHaEventWrapper != inputHaEventWrapper) { // See GEODE-4957
- entryMessage = (ClientUpdateMessageImpl) haContainer.get(inputHaEventWrapper);
addClientCQsAndInterestList(entryMessage, inputHaEventWrapper, haContainer,
regionName);
inputHaEventWrapper.setClientUpdateMessage(null);
newValueCd =
new VMCachedDeserializable(entryHaEventWrapper, newValueCd.getSizeInBytes());
+ } else {
+ entryHaEventWrapper = null;
}
- entryHaEventWrapper.setClientUpdateMessage(null);
- entryHaEventWrapper.setIsRefFromHAContainer(true);
+ }
+ } else { // putIfAbsent successful
+ synchronized (inputHaEventWrapper) {
+ inputHaEventWrapper.incAndGetReferenceCount();
+ inputHaEventWrapper.setHAContainer(haContainer);
+ inputHaEventWrapper.setClientUpdateMessage(null);
+ inputHaEventWrapper.setIsRefFromHAContainer(true);
}
break;
}
@@ -3443,76 +3426,55 @@ public class HARegionQueue implements RegionQueue {
*/
protected void putEventInHARegion(Conflatable event, Long position) {
if (event instanceof HAEventWrapper) {
- HAEventWrapper haEventWrapper = (HAEventWrapper) event;
+ HAEventWrapper inputHaEventWrapper = (HAEventWrapper) event;
if (this.isQueueInitialized()) {
- if (haEventWrapper.getIsRefFromHAContainer()) {
- putEntryConditionallyIntoHAContainer(haEventWrapper);
+ if (inputHaEventWrapper.getIsRefFromHAContainer()) {
+ putEntryConditionallyIntoHAContainer(inputHaEventWrapper);
} else {
- // This means that the haEvenWrapper reference we have is not
+ // This means that the haEventWrapper reference we have is not
// authentic, i.e. it doesn't refer to the HAEventWrapper instance
// in the haContainer, but to the one outside it.
- boolean entryFound;
- // synchronized (this.haContainer) {
- HAEventWrapper original = null;
+ HAEventWrapper haContainerKey = null;
do {
- ClientUpdateMessageImpl old =
+ ClientUpdateMessageImpl haContainerEntry =
(ClientUpdateMessageImpl) ((HAContainerWrapper) this.haContainer)
- .putIfAbsent(haEventWrapper, haEventWrapper.getClientUpdateMessage());
- if (old != null) {
- original =
- (HAEventWrapper) ((HAContainerWrapper) this.haContainer).getKey(haEventWrapper);
- if (original == null) {
+ .putIfAbsent(inputHaEventWrapper, inputHaEventWrapper.getClientUpdateMessage());
+ if (haContainerEntry != null) {
+ haContainerKey = (HAEventWrapper) ((HAContainerWrapper) this.haContainer)
+ .getKey(inputHaEventWrapper);
+ if (haContainerKey == null) {
continue;
}
- synchronized (original) {
+ synchronized (haContainerKey) {
// assert the entry is still present
- if (((HAContainerWrapper) this.haContainer).getKey(original) != null) {
- original.incAndGetReferenceCount();
- addClientCQsAndInterestList(old, haEventWrapper, this.haContainer,
- this.regionName);
- haEventWrapper = original;
+ if (((HAContainerWrapper) this.haContainer).getKey(haContainerKey) != null) {
+ haContainerKey.incAndGetReferenceCount();
+ addClientCQsAndInterestList(haContainerEntry, inputHaEventWrapper,
+ this.haContainer, this.regionName);
+ inputHaEventWrapper = haContainerKey;
} else {
- original = null;
+ haContainerKey = null;
}
}
} else {
- synchronized (haEventWrapper) {
- haEventWrapper.incAndGetReferenceCount();
- haEventWrapper.setHAContainer(this.haContainer);
- if (!haEventWrapper.getPutInProgress()) {
+ synchronized (inputHaEventWrapper) {
+ inputHaEventWrapper.incAndGetReferenceCount();
+ inputHaEventWrapper.setHAContainer(this.haContainer);
+ if (!inputHaEventWrapper.getPutInProgress()) {
// This means that this is a GII'ed event. Hence we must
// explicitly set 'clientUpdateMessage' to null.
- haEventWrapper.setClientUpdateMessage(null);
+ inputHaEventWrapper.setClientUpdateMessage(null);
}
- haEventWrapper.setIsRefFromHAContainer(true);
+ inputHaEventWrapper.setIsRefFromHAContainer(true);
}
break;
}
- } while (original == null);
- /*
- * entry = (Map.Entry)((HAContainerWrapper)this.haContainer) .getEntry(haEventWrapper); if
- * (entry == null) { entryFound = false;
- * putEntryConditionallyIntoHAContainer(haEventWrapper); } else { entryFound = true; // Do
- * not assign entry.getKey() to haEventWrapper right now.
- * ((HAEventWrapper)entry.getKey()).incAndGetReferenceCount(); } }//haContainer
- * synchronized ends if (entryFound) { addClientCQsAndInterestList(entry, haEventWrapper,
- * haContainer, regionName); haEventWrapper = (HAEventWrapper)entry.getKey(); } else { //
- * entry not found if (!haEventWrapper.getPutInProgress()) { // This means that this is a
- * GII'ed event. Hence we must // explicitly set 'clientUpdateMessage' to null.
- * haEventWrapper.setClientUpdateMessage(null); }
- * haEventWrapper.setIsRefFromHAContainer(true); }
- */
- }
- }
- // This has now been taken care of in AbstractRegionMap.initialImagePut()
- // else{
- // if(!haEventWrapper.getIsRefFromHAContainer()){
- // haEventWrapper =(HAEventWrapper)((HAContainerWrapper)haContainer).getKey(haEventWrapper);
- // }
- // }
+ } while (haContainerKey == null);
+ }
+ }
// Put the reference to the HAEventWrapper instance into the
// HA queue.
- this.region.put(position, haEventWrapper);
+ this.region.put(position, inputHaEventWrapper);
// logger.info(LocalizedStrings.DEBUG, "added message at position " + position);
} else { // (event instanceof ClientMarkerMessageImpl OR ConflatableObject OR
// ClientInstantiatorMessage)
--
To stop receiving notification emails like this one, please contact
ladyvader@apache.org.