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:07 UTC

[geode] branch feature/GEODE-5166 created (now ed3d10c)

This is an automated email from the ASF dual-hosted git repository.

ladyvader pushed a change to branch feature/GEODE-5166
in repository https://gitbox.apache.org/repos/asf/geode.git.


      at ed3d10c  GEODE-5166: NPE thrown while processing InitialImage of subscription region

This branch includes the following new commits:

     new ed3d10c  GEODE-5166: NPE thrown while processing InitialImage of subscription region

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


-- 
To stop receiving notification emails like this one, please contact
ladyvader@apache.org.

[geode] 01/01: GEODE-5166: NPE thrown while processing InitialImage of subscription region

Posted by la...@apache.org.
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.