Repository: hadoop Updated Branches: refs/heads/trunk e597249d3 -> a095622f3
HADOOP-10852 Fix thread safety issues in NetgroupCache. (Benoy Antony) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/a095622f Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/a095622f Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/a095622f Branch: refs/heads/trunk Commit: a095622f36c5e9fff3ec02b14b800038a81f6286 Parents: e597249 Author: Benoy Antony <[email protected]> Authored: Mon Dec 15 14:00:25 2014 -0800 Committer: Benoy Antony <[email protected]> Committed: Mon Dec 15 14:00:25 2014 -0800 ---------------------------------------------------------------------- .../apache/hadoop/security/NetgroupCache.java | 61 +++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/a095622f/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java index bd9c448..4495a66 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java @@ -17,11 +17,11 @@ */ package org.apache.hadoop.security; +import java.util.Collections; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.HashSet; import java.util.concurrent.ConcurrentHashMap; import org.apache.hadoop.classification.InterfaceAudience; @@ -36,14 +36,9 @@ import org.apache.hadoop.classification.InterfaceStability; @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) @InterfaceStability.Unstable public class NetgroupCache { - private static boolean netgroupToUsersMapUpdated = true; - private static Map<String, Set<String>> netgroupToUsersMap = - new ConcurrentHashMap<String, Set<String>>(); - - private static Map<String, Set<String>> userToNetgroupsMap = + private static ConcurrentHashMap<String, Set<String>> userToNetgroupsMap = new ConcurrentHashMap<String, Set<String>>(); - /** * Get netgroups for a given user * @@ -52,21 +47,11 @@ public class NetgroupCache { */ public static void getNetgroups(final String user, List<String> groups) { - if(netgroupToUsersMapUpdated) { - netgroupToUsersMapUpdated = false; // at the beginning to avoid race - //update userToNetgroupsMap - for(String netgroup : netgroupToUsersMap.keySet()) { - for(String netuser : netgroupToUsersMap.get(netgroup)) { - // add to userToNetgroupsMap - if(!userToNetgroupsMap.containsKey(netuser)) { - userToNetgroupsMap.put(netuser, new HashSet<String>()); - } - userToNetgroupsMap.get(netuser).add(netgroup); - } - } - } - if(userToNetgroupsMap.containsKey(user)) { - groups.addAll(userToNetgroupsMap.get(user)); + Set<String> userGroups = userToNetgroupsMap.get(user); + //ConcurrentHashMap does not allow null values; + //So null value check can be used to check if the key exists + if (userGroups != null) { + groups.addAll(userGroups); } } @@ -76,7 +61,15 @@ public class NetgroupCache { * @return list of cached groups */ public static List<String> getNetgroupNames() { - return new LinkedList<String>(netgroupToUsersMap.keySet()); + return new LinkedList<String>(getGroups()); + } + + private static Set<String> getGroups() { + Set<String> allGroups = new HashSet<String> (); + for (Set<String> userGroups : userToNetgroupsMap.values()) { + allGroups.addAll(userGroups); + } + return allGroups; } /** @@ -86,14 +79,13 @@ public class NetgroupCache { * @return true if group is cached, false otherwise */ public static boolean isCached(String group) { - return netgroupToUsersMap.containsKey(group); + return getGroups().contains(group); } /** * Clear the cache */ public static void clear() { - netgroupToUsersMap.clear(); userToNetgroupsMap.clear(); } @@ -104,7 +96,20 @@ public class NetgroupCache { * @param users list of users for a given group */ public static void add(String group, List<String> users) { - netgroupToUsersMap.put(group, new HashSet<String>(users)); - netgroupToUsersMapUpdated = true; // at the end to avoid race + for (String user : users) { + Set<String> userGroups = userToNetgroupsMap.get(user); + // ConcurrentHashMap does not allow null values; + // So null value check can be used to check if the key exists + if (userGroups == null) { + //Generate a ConcurrentHashSet (backed by the keyset of the ConcurrentHashMap) + userGroups = + Collections.newSetFromMap(new ConcurrentHashMap<String,Boolean>()); + Set<String> currentSet = userToNetgroupsMap.putIfAbsent(user, userGroups); + if (currentSet != null) { + userGroups = currentSet; + } + } + userGroups.add(group); + } } }
