HADOOP-8050. Deadlock in metrics. Contributed by Kihwal Lee.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1291084 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -212,6 +212,8 @@ Release 0.23.2 - UNRELEASED
     HADOOP-7680 TestHardLink fails on Mac OS X, when gnu stat is in path.
     (Milind Bhandarkar via stevel)
 
+    HADOOP-8050. Deadlock in metrics. (Kihwal Lee via mattf)
+
 Release 0.23.1 - 2012-02-17 
 
   INCOMPATIBLE CHANGES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/impl/MetricsSourceAdapter.java

@@ -94,9 +94,10 @@ class MetricsSourceAdapter implements DynamicMBean {
   }
 
   @Override
-  public synchronized Object getAttribute(String attribute)
+  public Object getAttribute(String attribute)
       throws AttributeNotFoundException, MBeanException, ReflectionException {
     updateJmxCache();
+    synchronized(this) {
       Attribute a = attrCache.get(attribute);
       if (a == null) {
         throw new AttributeNotFoundException(attribute +" not found");
@@ -106,6 +107,7 @@ class MetricsSourceAdapter implements DynamicMBean {
       }
       return a.getValue();
     }
+  }
 
   @Override
   public void setAttribute(Attribute attribute)
@@ -115,8 +117,9 @@ class MetricsSourceAdapter implements DynamicMBean {
   }
 
   @Override
-  public synchronized AttributeList getAttributes(String[] attributes) {
+  public AttributeList getAttributes(String[] attributes) {
     updateJmxCache();
+    synchronized(this) {
       AttributeList ret = new AttributeList();
       for (String key : attributes) {
         Attribute attr = attrCache.get(key);
@@ -127,6 +130,7 @@ class MetricsSourceAdapter implements DynamicMBean {
       }
       return ret;
     }
+  }
 
   @Override
   public AttributeList setAttributes(AttributeList attributes) {
@@ -140,17 +144,32 @@ class MetricsSourceAdapter implements DynamicMBean {
   }
 
   @Override
-  public synchronized MBeanInfo getMBeanInfo() {
+  public MBeanInfo getMBeanInfo() {
     updateJmxCache();
     return infoCache;
   }
 
-  private synchronized void updateJmxCache() {
+  private void updateJmxCache() {
+    boolean getAllMetrics = false;
+    synchronized(this) {
       if (System.currentTimeMillis() - jmxCacheTS >= jmxCacheTTL) {
+        // temporarilly advance the expiry while updating the cache
+        jmxCacheTS = System.currentTimeMillis() + jmxCacheTTL;
         if (lastRecs == null) {
+          getAllMetrics = true;
+        }
+      }
+      else {
+        return;
+      }
+    }
+
+    if (getAllMetrics) {
       MetricsCollectorImpl builder = new MetricsCollectorImpl();
       getMetrics(builder, true);
     }
+
+    synchronized(this) {
       int oldCacheSize = attrCache.size();
       int newCacheSize = updateAttrCache();
       if (oldCacheSize < newCacheSize) {