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 aed333beeda12fb1bdb6dba7cb620e3cfee6081a Author: Lynn Hughes-Godfrey <lhughesgodf...@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 ladyva...@apache.org.