From 4d1007d75c24f4e9f1d8df18cb3faae53b183661 Mon Sep 17 00:00:00 2001
From: Alexander Morozov <lk4d4@docker.com>
Date: Tue, 1 Dec 2015 09:54:26 -0800
Subject: [PATCH] Fix race between two ContainerRm

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
---
 daemon/create.go      |  2 +-
 daemon/delete.go      | 25 +++++++++++++++----------
 daemon/delete_test.go |  5 ++++-
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/daemon/create.go b/daemon/create.go
index 484952108a..46c6643c54 100644
--- a/daemon/create.go
+++ b/daemon/create.go
@@ -76,7 +76,7 @@ func (daemon *Daemon) create(params *ContainerCreateConfig) (retC *Container, re
 	}
 	defer func() {
 		if retErr != nil {
-			if err := daemon.rm(container, true); err != nil {
+			if err := daemon.ContainerRm(container.ID, &ContainerRmConfig{ForceRemove: true}); err != nil {
 				logrus.Errorf("Clean up Error! Cannot destroy container %s: %v", container.ID, err)
 			}
 		}
diff --git a/daemon/delete.go b/daemon/delete.go
index 08b3b0157c..12f614cb12 100644
--- a/daemon/delete.go
+++ b/daemon/delete.go
@@ -25,6 +25,21 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error
 		return err
 	}
 
+	// Container state RemovalInProgress should be used to avoid races.
+	if err = container.setRemovalInProgress(); err != nil {
+		if err == derr.ErrorCodeAlreadyRemoving {
+			// do not fail when the removal is in progress started by other request.
+			return nil
+		}
+		return derr.ErrorCodeRmState.WithArgs(err)
+	}
+	defer container.resetRemovalInProgress()
+
+	// check if container wasn't deregistered by previous rm since Get
+	if c := daemon.containers.Get(container.ID); c == nil {
+		return nil
+	}
+
 	if config.RemoveLink {
 		name, err := GetFullContainerName(name)
 		if err != nil {
@@ -76,16 +91,6 @@ func (daemon *Daemon) rm(container *Container, forceRemove bool) (err error) {
 		}
 	}
 
-	// Container state RemovalInProgress should be used to avoid races.
-	if err = container.setRemovalInProgress(); err != nil {
-		if err == derr.ErrorCodeAlreadyRemoving {
-			// do not fail when the removal is in progress started by other request.
-			return nil
-		}
-		return derr.ErrorCodeRmState.WithArgs(err)
-	}
-	defer container.resetRemovalInProgress()
-
 	// stop collection of stats for the container regardless
 	// if stats are currently getting collected.
 	daemon.statsCollector.stopCollection(container)
diff --git a/daemon/delete_test.go b/daemon/delete_test.go
index 9a82fce2d2..d2fd765a3e 100644
--- a/daemon/delete_test.go
+++ b/daemon/delete_test.go
@@ -18,13 +18,16 @@ func TestContainerDoubleDelete(t *testing.T) {
 		repository: tmp,
 		root:       tmp,
 	}
+	daemon.containers = &contStore{s: make(map[string]*Container)}
 
 	container := &Container{
 		CommonContainer: CommonContainer{
+			ID:     "test",
 			State:  NewState(),
 			Config: &runconfig.Config{},
 		},
 	}
+	daemon.containers.Add(container.ID, container)
 
 	// Mark the container as having a delete in progress
 	if err := container.setRemovalInProgress(); err != nil {
@@ -33,7 +36,7 @@ func TestContainerDoubleDelete(t *testing.T) {
 
 	// Try to remove the container when it's start is removalInProgress.
 	// It should ignore the container and not return an error.
-	if err := daemon.rm(container, true); err != nil {
+	if err := daemon.ContainerRm(container.ID, &ContainerRmConfig{ForceRemove: true}); err != nil {
 		t.Fatal(err)
 	}
 }