MAPREDUCE-4359. Potential deadlock in Counters.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-1@1357831 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -56,6 +56,8 @@ Release 1.2.0 - unreleased
     MAPREDUCE-4385. FairScheduler.maxTasksToAssign() should check for 
     fairscheduler.assignmultiple.maps < TaskTracker.availableSlots (kkambatl via tucu)
 
+    MAPREDUCE-4359. Potential deadlock in Counters. (tomwhite)
+
 Release 1.1.0 - unreleased
 
   INCOMPATIBLE CHANGES

src/mapred/org/apache/hadoop/mapred/Counters.java

@@ -295,7 +295,7 @@ public class Counters implements Writable, Iterable<Counters.Group> {
      * @deprecated use {@link #getCounter(String)} instead
      */
     @Deprecated
-    public synchronized Counter getCounter(int id, String name) {
+    public Counter getCounter(int id, String name) {
       return getCounterForName(name);
     }
     
@@ -304,23 +304,27 @@ public class Counters implements Writable, Iterable<Counters.Group> {
      * @param name the internal counter name
      * @return the counter
      */
-    public synchronized Counter getCounterForName(String name) {
-      String shortName = getShortName(name, COUNTER_NAME_LIMIT);
-      Counter result = subcounters.get(shortName);
-      if (result == null) {
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Adding " + shortName);
-        }
-        numCounters = (numCounters == 0) ? Counters.this.size(): numCounters; 
-        if (numCounters >= MAX_COUNTER_LIMIT) {
-          throw new CountersExceededException("Error: Exceeded limits on number of counters - " 
-              + "Counters=" + numCounters + " Limit=" + MAX_COUNTER_LIMIT);
+    public Counter getCounterForName(String name) {
+      synchronized(Counters.this) { // lock ordering: Counters then Group
+        synchronized (this) {
+          String shortName = getShortName(name, COUNTER_NAME_LIMIT);
+          Counter result = subcounters.get(shortName);
+          if (result == null) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug("Adding " + shortName);
+            }
+            numCounters = (numCounters == 0) ? Counters.this.size(): numCounters; 
+            if (numCounters >= MAX_COUNTER_LIMIT) {
+              throw new CountersExceededException("Error: Exceeded limits on number of counters - " 
+                  + "Counters=" + numCounters + " Limit=" + MAX_COUNTER_LIMIT);
+            }
+            result = new Counter(shortName, localize(shortName + ".name", shortName), 0L);
+            subcounters.put(shortName, result);
+            numCounters++;
+          }
+          return result;
         }
-        result = new Counter(shortName, localize(shortName + ".name", shortName), 0L);
-        subcounters.put(shortName, result);
-        numCounters++;
       }
-      return result;
     }
     
     /**