1. docker
  2. Commit 2f16895ee94848e2d8ad72bc01968b4c88d84cb8
Browse code

devmapper: Drop devices lock before returning from function

cleanupDeleted() takes devices.Lock() but does not drop it if there are
no deleted devices. Hence docker deadlocks if one is using deferred
device deletion feature. (--storage-opt dm.use_deferred_deletion=true).

Fix it. Drop the lock before returning.

Also added a unit test case to make sure in future this can be easily
detected if somebody changes the function.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Vivek Goyal authored on 2015/10/20 06:51:17
Showing 2 changed files
daemon/graphdriver/devmapper/deviceset.go
History View file @ 2f16895
... ... 
@@ -599,6 +599,7 @@ func (devices *DeviceSet) cleanupDeletedDevices() error {
599 599
600 600 
 	// If there are no deleted devices, there is nothing to do.
601 601 
 	if devices.nrDeletedDevices == 0 {
602 
+		devices.Unlock()
602 603 
 		return nil
603 604 
 	}
604 605
daemon/graphdriver/devmapper/devmapper_test.go
History View file @ 2f16895
... ... 
@@ -5,6 +5,7 @@ package devmapper
5 5 
 import (
6 6 
 	"fmt"
7 7 
 	"testing"
8 
+	"time"
8 9
9 10 
 	"github.com/docker/docker/daemon/graphdriver"
10 11 
 	"github.com/docker/docker/daemon/graphdriver/graphtest"
... ... 
@@ -79,3 +80,31 @@ func testChangeLoopBackSize(t *testing.T, delta, expectDataSize, expectMetaDataS
79 79 
 		t.Fatal(err)
80 80 
 	}
81 81 
 }
82 
+
83 
+// Make sure devices.Lock() has been release upon return from cleanupDeletedDevices() function
84 
+func TestDevmapperLockReleasedDeviceDeletion(t *testing.T) {
85 
+	driver := graphtest.GetDriver(t, "devicemapper").(*graphtest.Driver).Driver.(*graphdriver.NaiveDiffDriver).ProtoDriver.(*Driver)
86 
+	defer graphtest.PutDriver(t)
87 
+
88 
+	// Call cleanupDeletedDevices() and after the call take and release
89 
+	// DeviceSet Lock. If lock has not been released, this will hang.
90 
+	driver.DeviceSet.cleanupDeletedDevices()
91 
+
92 
+	doneChan := make(chan bool)
93 
+
94 
+	go func() {
95 
+		driver.DeviceSet.Lock()
96 
+		defer driver.DeviceSet.Unlock()
97 
+		doneChan <- true
98 
+	}()
99 
+
100 
+	select {
101 
+	case <-time.After(time.Second * 5):
102 
+		// Timer expired. That means lock was not released upon
103 
+		// function return and we are deadlocked. Release lock
104 
+		// here so that cleanup could succeed and fail the test.
105 
+		driver.DeviceSet.Unlock()
106 
+		t.Fatalf("Could not acquire devices lock after call to cleanupDeletedDevices()")
107 
+	case <-doneChan:
108 
+	}
109 
+}