From 9d4dedd65faebfedfbb7632f89c274ef8b08dc4b Mon Sep 17 00:00:00 2001
From: Rohith Sharma K S <rohithsharmaks@apache.org>
Date: Tue, 12 Apr 2016 09:59:33 +0530
Subject: [PATCH] YARN-4794. Deadlock in NMClientImpl. Contributed by Jian He.

(cherry picked from commit ff722bbbdfaf9715643507628e94ece26d904b9d)
---
 .../yarn/client/api/impl/NMClientImpl.java    | 36 ++++---------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/NMClientImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/NMClientImpl.java
index e047368c458..dc92cda3d5a 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/NMClientImpl.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/NMClientImpl.java
@@ -171,8 +171,6 @@ public class NMClientImpl extends NMClient {
       throw RPCUtil.getRemoteException("Container "
           + startedContainer.containerId.toString() + " is already started");
     }
-    startedContainers
-        .put(startedContainer.getContainerId(), startedContainer);
   }
 
   @Override
@@ -182,7 +180,8 @@ public class NMClientImpl extends NMClient {
     // Do synchronization on StartedContainer to prevent race condition
     // between startContainer and stopContainer only when startContainer is
     // in progress for a given container.
-    StartedContainer startingContainer = createStartedContainer(container);
+    StartedContainer startingContainer =
+        new StartedContainer(container.getId(), container.getNodeId());
     synchronized (startingContainer) {
       addStartingContainer(startingContainer);
       
@@ -210,18 +209,14 @@ public class NMClientImpl extends NMClient {
         }
         allServiceResponse = response.getAllServicesMetaData();
         startingContainer.state = ContainerState.RUNNING;
-      } catch (YarnException e) {
+      } catch (YarnException | IOException e) {
         startingContainer.state = ContainerState.COMPLETE;
         // Remove the started container if it failed to start
-        removeStartedContainer(startingContainer);
-        throw e;
-      } catch (IOException e) {
-        startingContainer.state = ContainerState.COMPLETE;
-        removeStartedContainer(startingContainer);
+        startedContainers.remove(startingContainer.containerId);
         throw e;
       } catch (Throwable t) {
         startingContainer.state = ContainerState.COMPLETE;
-        removeStartedContainer(startingContainer);
+        startedContainers.remove(startingContainer.containerId);
         throw RPCUtil.getRemoteException(t);
       } finally {
         if (proxy != null) {
@@ -263,7 +258,7 @@ public class NMClientImpl extends NMClient {
   @Override
   public void stopContainer(ContainerId containerId, NodeId nodeId)
       throws YarnException, IOException {
-    StartedContainer startedContainer = getStartedContainer(containerId);
+    StartedContainer startedContainer = startedContainers.get(containerId);
 
     // Only allow one request of stopping the container to move forward
     // When entering the block, check whether the precursor has already stopped
@@ -276,7 +271,7 @@ public class NMClientImpl extends NMClient {
         stopContainerInternal(containerId, nodeId);
         // Only after successful
         startedContainer.state = ContainerState.COMPLETE;
-        removeStartedContainer(startedContainer);
+        startedContainers.remove(startedContainer.containerId);
       }
     } else {
       stopContainerInternal(containerId, nodeId);
@@ -333,23 +328,6 @@ public class NMClientImpl extends NMClient {
       }
     }
   }
-  
-  protected synchronized StartedContainer createStartedContainer(
-      Container container) throws YarnException, IOException {
-    StartedContainer startedContainer = new StartedContainer(container.getId(),
-        container.getNodeId());
-    return startedContainer;
-  }
-
-  protected synchronized void
-      removeStartedContainer(StartedContainer container) {
-    startedContainers.remove(container.containerId);
-  }
-
-  protected synchronized StartedContainer getStartedContainer(
-      ContainerId containerId) {
-    return startedContainers.get(containerId);
-  }
 
   public AtomicBoolean getCleanupRunningContainers() {
     return cleanupRunningContainers;