Merge pull request #2355 from tanenbaum/bug/container_deadlock

Fix container `OnDemandHousekeeping` blocking on stop channel.

Author: dashpole

manager/container.go

@@ -81,7 +81,7 @@ type containerData struct {
 	logUsage bool
 
 	// Tells the container to stop.
-	stop chan bool
+	stop chan struct{}
 
 	// Tells the container to immediately collect stats
 	onDemandChan chan chan struct{}
@@ -114,7 +114,7 @@ func (c *containerData) Stop() error {
 	if err != nil {
 		return err
 	}
-	c.stop <- true
+	close(c.stop)
 	return nil
 }
 
@@ -383,7 +383,7 @@ func newContainerData(containerName string, memoryCache *memory.InMemoryCache, h
 		allowDynamicHousekeeping: allowDynamicHousekeeping,
 		logUsage:                 logUsage,
 		loadAvg:                  -1.0, // negative value indicates uninitialized.
-		stop:                     make(chan bool, 1),
+		stop:                     make(chan struct{}),
 		collectorManager:         collectorManager,
 		onDemandChan:             make(chan chan struct{}, 100),
 		clock:                    clock,

manager/container_test.go

@@ -287,3 +287,28 @@ func TestConcurrentOnDemandHousekeeping(t *testing.T) {
 	checkNumStats(t, memoryCache, 1)
 	mockHandler.AssertExpectations(t)
 }
+
+func TestOnDemandHousekeepingReturnsAfterStopped(t *testing.T) {
+	statsList := itest.GenerateRandomStats(1, 4, 1*time.Second)
+	stats := statsList[0]
+
+	cd, mockHandler, memoryCache, fakeClock := newTestContainerData(t)
+	mockHandler.On("GetStats").Return(stats, nil)
+
+	// trigger housekeeping update
+	go cd.OnDemandHousekeeping(0 * time.Second)
+	cd.housekeepingTick(fakeClock.NewTimer(time.Minute).C(), testLongHousekeeping)
+
+	checkNumStats(t, memoryCache, 1)
+
+	fakeClock.Step(2 * time.Second)
+
+	cd.Stop()
+	// housekeeping tick should detect stop and not store any more metrics
+	assert.False(t, cd.housekeepingTick(fakeClock.NewTimer(time.Minute).C(), testLongHousekeeping))
+	fakeClock.Step(1 * time.Second)
+	// on demand housekeeping should not block and return
+	cd.OnDemandHousekeeping(-1 * time.Second)
+
+	mockHandler.AssertExpectations(t)
+}