[GitHub] [geode] dschneider-pivotal commented on a change in pull request #5823: GEODE-8771: invalidate should acquire the lock before initIndex

[GitBox <gi...@apache.org>]

dschneider-pivotal commented on a change in pull request #5823:
URL: https://github.com/apache/geode/pull/5823#discussion_r539607568



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
##########
@@ -1282,14 +1282,18 @@ public boolean invalidate(EntryEventImpl event, boolean invokeCallbacks, boolean
     DiskRegion dr = owner.getDiskRegion();
     boolean ownerIsInitialized = owner.isInitialized();
 
+    // lock before waitForIndexInit so that we should wait
+    // till a concurrent clear to finish
+    lockForCacheModification(owner, event);

Review comment:
       Since you moved this up to be before "waitForIndexInit" is called you need to add a try/finally that started right after lockForCacheModification returns and the finally should be way down past the finally for the try that started right after "lockWhenRegionIsInitializing" returns. If you look at the finally for that try you will see it calls releaseCacheModificationLock. This needs to instead be called in your new finally block so that it corresponds to the lockForCacheModification call.
   It should have probably been done this way already (two finally blocks; one that calls releaseCacheModificationLock and another that calls unlockWhenRegionIsInitializing) but it is needed now because waitForIndexInit can throw an exception because it calls checkCancelInProgress.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org