geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ladyva...@apache.org
Subject [geode] branch develop updated: GEODE-5166: NPE thrown while processing InitialImage of subscription region
Date Mon, 07 May 2018 20:12:30 GMT
This is an automated email from the ASF dual-hosted git repository.

ladyvader 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 1dfbffa  GEODE-5166: NPE thrown while processing InitialImage of subscription region
1dfbffa is described below

commit 1dfbffa71a377aa276729c80c35f0c25a2886b25
Author: Lynn Hughes-Godfrey <lhughesgodfrey@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.

Mime
View raw message