Browse code

Fix race between two ContainerRm

Signed-off-by: Alexander Morozov <lk4d4@docker.com>

Alexander Morozov authored on 2015/12/02 02:54:26
Showing 3 changed files
... ...
@@ -76,7 +76,7 @@ func (daemon *Daemon) create(params *ContainerCreateConfig) (retC *Container, re
76 76
 	}
77 77
 	defer func() {
78 78
 		if retErr != nil {
79
-			if err := daemon.rm(container, true); err != nil {
79
+			if err := daemon.ContainerRm(container.ID, &ContainerRmConfig{ForceRemove: true}); err != nil {
80 80
 				logrus.Errorf("Clean up Error! Cannot destroy container %s: %v", container.ID, err)
81 81
 			}
82 82
 		}
... ...
@@ -25,6 +25,21 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error
25 25
 		return err
26 26
 	}
27 27
 
28
+	// Container state RemovalInProgress should be used to avoid races.
29
+	if err = container.setRemovalInProgress(); err != nil {
30
+		if err == derr.ErrorCodeAlreadyRemoving {
31
+			// do not fail when the removal is in progress started by other request.
32
+			return nil
33
+		}
34
+		return derr.ErrorCodeRmState.WithArgs(err)
35
+	}
36
+	defer container.resetRemovalInProgress()
37
+
38
+	// check if container wasn't deregistered by previous rm since Get
39
+	if c := daemon.containers.Get(container.ID); c == nil {
40
+		return nil
41
+	}
42
+
28 43
 	if config.RemoveLink {
29 44
 		name, err := GetFullContainerName(name)
30 45
 		if err != nil {
... ...
@@ -76,16 +91,6 @@ func (daemon *Daemon) rm(container *Container, forceRemove bool) (err error) {
76 76
 		}
77 77
 	}
78 78
 
79
-	// Container state RemovalInProgress should be used to avoid races.
80
-	if err = container.setRemovalInProgress(); err != nil {
81
-		if err == derr.ErrorCodeAlreadyRemoving {
82
-			// do not fail when the removal is in progress started by other request.
83
-			return nil
84
-		}
85
-		return derr.ErrorCodeRmState.WithArgs(err)
86
-	}
87
-	defer container.resetRemovalInProgress()
88
-
89 79
 	// stop collection of stats for the container regardless
90 80
 	// if stats are currently getting collected.
91 81
 	daemon.statsCollector.stopCollection(container)
... ...
@@ -18,13 +18,16 @@ func TestContainerDoubleDelete(t *testing.T) {
18 18
 		repository: tmp,
19 19
 		root:       tmp,
20 20
 	}
21
+	daemon.containers = &contStore{s: make(map[string]*Container)}
21 22
 
22 23
 	container := &Container{
23 24
 		CommonContainer: CommonContainer{
25
+			ID:     "test",
24 26
 			State:  NewState(),
25 27
 			Config: &runconfig.Config{},
26 28
 		},
27 29
 	}
30
+	daemon.containers.Add(container.ID, container)
28 31
 
29 32
 	// Mark the container as having a delete in progress
30 33
 	if err := container.setRemovalInProgress(); err != nil {
... ...
@@ -33,7 +36,7 @@ func TestContainerDoubleDelete(t *testing.T) {
33 33
 
34 34
 	// Try to remove the container when it's start is removalInProgress.
35 35
 	// It should ignore the container and not return an error.
36
-	if err := daemon.rm(container, true); err != nil {
36
+	if err := daemon.ContainerRm(container.ID, &ContainerRmConfig{ForceRemove: true}); err != nil {
37 37
 		t.Fatal(err)
38 38
 	}
39 39
 }