Browse Source

Merge pull request #17176 from rhvgoyal/fix-locking-issue

devmapper: Drop devices lock before returning from function
Brian Goff 9 years ago
parent
commit
7777c1be9b

+ 1 - 0
daemon/graphdriver/devmapper/deviceset.go

@@ -599,6 +599,7 @@ func (devices *DeviceSet) cleanupDeletedDevices() error {
 
 	// If there are no deleted devices, there is nothing to do.
 	if devices.nrDeletedDevices == 0 {
+		devices.Unlock()
 		return nil
 	}
 

+ 29 - 0
daemon/graphdriver/devmapper/devmapper_test.go

@@ -5,6 +5,7 @@ package devmapper
 import (
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/daemon/graphdriver/graphtest"
@@ -79,3 +80,31 @@ func testChangeLoopBackSize(t *testing.T, delta, expectDataSize, expectMetaDataS
 		t.Fatal(err)
 	}
 }
+
+// Make sure devices.Lock() has been release upon return from cleanupDeletedDevices() function
+func TestDevmapperLockReleasedDeviceDeletion(t *testing.T) {
+	driver := graphtest.GetDriver(t, "devicemapper").(*graphtest.Driver).Driver.(*graphdriver.NaiveDiffDriver).ProtoDriver.(*Driver)
+	defer graphtest.PutDriver(t)
+
+	// Call cleanupDeletedDevices() and after the call take and release
+	// DeviceSet Lock. If lock has not been released, this will hang.
+	driver.DeviceSet.cleanupDeletedDevices()
+
+	doneChan := make(chan bool)
+
+	go func() {
+		driver.DeviceSet.Lock()
+		defer driver.DeviceSet.Unlock()
+		doneChan <- true
+	}()
+
+	select {
+	case <-time.After(time.Second * 5):
+		// Timer expired. That means lock was not released upon
+		// function return and we are deadlocked. Release lock
+		// here so that cleanup could succeed and fail the test.
+		driver.DeviceSet.Unlock()
+		t.Fatalf("Could not acquire devices lock after call to cleanupDeletedDevices()")
+	case <-doneChan:
+	}
+}