mneethiraj commented on code in PR #538:
URL: https://github.com/apache/ranger/pull/538#discussion_r1980165272


##########
security-admin/src/main/java/org/apache/ranger/common/RangerRoleCache.java:
##########
@@ -69,21 +70,42 @@ public static RangerRoleCache getInstance() {
     }
 
     public RangerRoles getLatestRangerRoleOrCached(String serviceName, 
RoleDBStore roleDBStore, Long lastKnownRoleVersion, Long rangerRoleVersionInDB) 
throws Exception {
-        final RangerRoles ret;
+        RangerRoles ret = null;
 
         if (lastKnownRoleVersion == null || 
!lastKnownRoleVersion.equals(rangerRoleVersionInDB)) {
-            roleCacheWrapper = new RangerRoleCacheWrapper();
-            ret              = 
roleCacheWrapper.getLatestRangerRoles(serviceName, roleDBStore, 
lastKnownRoleVersion, rangerRoleVersionInDB);
-        } else {
-            ret = null;
+            ret = roleCacheWrapper.getRoles();
+
+            if (roleCacheWrapper.getRolesVersion() < rangerRoleVersionInDB) {
+                boolean lockResult = false;
+                try {
+                    lockResult = lock.tryLock(waitTimeInSeconds, 
TimeUnit.SECONDS);
+
+                    if (lockResult) {
+                        if (roleCacheWrapper.getRolesVersion() < 
rangerRoleVersionInDB) {
+                            ret = roleCacheWrapper.getLatestRangerRoles(
+                                    serviceName, roleDBStore, 
lastKnownRoleVersion, rangerRoleVersionInDB);
+                        }

Review Comment:
   `ret` initialized in line 76 is an older version, hence the `if` block at 
line 78 is entered. Consider 2 threads:
   1. thread-1: `ret` is initialized to version=100; acquired lock at line 81
   2. thread-2: `ret` is initialized to version=100; waiting for the lock at 
line 81
   3. thread-1: refreshes `roleCacheWrapper` and `ret` to the latest version, 
say 101, at line 85; releases the lock
   4. thread-2: acquires the lock, finds that `roleCacheWrapper` has the 
lastest version; skips line 85; leaves `ret` at version 100
   
   The call from thread-2 will return version 100; the expected version is 101. 
Makes sense?



-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@ranger.apache.org

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

Reply via email to