Merge pull request #1051 from jimmidyson/fs-goroutine-leaks

Fix FS usage goroutine leaks

container/container.go

@@ -81,4 +81,8 @@ type ContainerHandler interface {
 
 	// Cleanup frees up any resources being held like fds or go routines, etc.
 	Cleanup()
+
+	// Start starts any necessary background goroutines - must be cleaned up in Cleanup().
+	// It is expected that most implementations will be a no-op.
+	Start()
 }

container/docker/handler.go

@@ -142,9 +142,6 @@ func newDockerContainerHandler(
 		fsHandler:          newFsHandler(time.Minute, storageDirs, fsInfo),
 	}
 
-	// Start the filesystem handler.
-	handler.fsHandler.start()
-
 	// We assume that if Inspect fails then the container is not known to docker.
 	ctnr, err := client.InspectContainer(id)
 	if err != nil {
@@ -172,6 +169,11 @@ func newDockerContainerHandler(
 	return handler, nil
 }
 
+func (self *dockerContainerHandler) Start() {
+	// Start the filesystem handler.
+	self.fsHandler.start()
+}
+
 func (self *dockerContainerHandler) Cleanup() {
 	self.fsHandler.stop()
 }

container/mock.go

@@ -51,6 +51,8 @@ func (self *MockContainerHandler) ContainerReference() (info.ContainerReference,
 	return args.Get(0).(info.ContainerReference), args.Error(1)
 }
 
+func (self *MockContainerHandler) Start() {}
+
 func (self *MockContainerHandler) Cleanup() {}
 
 func (self *MockContainerHandler) GetSpec() (info.ContainerSpec, error) {

container/raw/handler.go

@@ -166,6 +166,9 @@ func (self *rawContainerHandler) GetRootNetworkDevices() ([]info.NetInfo, error)
 	return nd, nil
 }
 
+// Nothing to start up.
+func (self *rawContainerHandler) Start() {}
+
 // Nothing to clean up.
 func (self *rawContainerHandler) Cleanup() {}
 

manager/container.go

@@ -361,6 +361,9 @@ func (self *containerData) nextHousekeeping(lastHousekeeping time.Time) time.Tim
 
 // TODO(vmarmol): Implement stats collecting as a custom collector.
 func (c *containerData) housekeeping() {
+	// Start any background goroutines - must be cleaned up in c.handler.Cleanup().
+	c.handler.Start()
+
 	// Long housekeeping is either 100ms or half of the housekeeping interval.
 	longHousekeeping := 100 * time.Millisecond
 	if *HousekeepingInterval/2 < longHousekeeping {

manager/manager.go

@@ -746,6 +746,18 @@ func (m *manager) registerCollectors(collectorConfigs map[string]string, cont *c
 
 // Create a container.
 func (m *manager) createContainer(containerName string) error {
+	m.containersLock.Lock()
+	defer m.containersLock.Unlock()
+
+	namespacedName := namespacedContainerName{
+		Name: containerName,
+	}
+
+	// Check that the container didn't already exist.
+	if _, ok := m.containers[namespacedName]; ok {
+		return nil
+	}
+
 	handler, accept, err := container.NewContainerHandler(containerName, m.inHostNamespace)
 	if err != nil {
 		return err
@@ -775,35 +787,15 @@ func (m *manager) createContainer(containerName string) error {
 		return err
 	}
 
-	// Add to the containers map.
-	alreadyExists := func() bool {
-		m.containersLock.Lock()
-		defer m.containersLock.Unlock()
-
-		namespacedName := namespacedContainerName{
-			Name: containerName,
-		}
-
-		// Check that the container didn't already exist.
-		_, ok := m.containers[namespacedName]
-		if ok {
-			return true
-		}
-
-		// Add the container name and all its aliases. The aliases must be within the namespace of the factory.
-		m.containers[namespacedName] = cont
-		for _, alias := range cont.info.Aliases {
-			m.containers[namespacedContainerName{
-				Namespace: cont.info.Namespace,
-				Name:      alias,
-			}] = cont
-		}
-
-		return false
-	}()
-	if alreadyExists {
-		return nil
+	// Add the container name and all its aliases. The aliases must be within the namespace of the factory.
+	m.containers[namespacedName] = cont
+	for _, alias := range cont.info.Aliases {
+		m.containers[namespacedContainerName{
+			Namespace: cont.info.Namespace,
+			Name:      alias,
+		}] = cont
 	}
+
 	glog.V(3).Infof("Added container: %q (aliases: %v, namespace: %q)", containerName, cont.info.Aliases, cont.info.Namespace)
 
 	contSpec, err := cont.handler.GetSpec()
@@ -827,9 +819,7 @@ func (m *manager) createContainer(containerName string) error {
 	}
 
 	// Start the container's housekeeping.
-	cont.Start()
-
-	return nil
+	return cont.Start()
 }
 
 func (m *manager) destroyContainer(containerName string) error {