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