Skip to content

Commit

Permalink
Fix FS usage goroutine leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
jimmidyson committed Jan 14, 2016
1 parent b47498c commit 09f948c
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 32 deletions.
4 changes: 4 additions & 0 deletions container/container.go
Expand Up @@ -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()
}
8 changes: 5 additions & 3 deletions container/docker/handler.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand Down
2 changes: 2 additions & 0 deletions container/mock.go
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions container/raw/handler.go
Expand Up @@ -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() {}

Expand Down
3 changes: 3 additions & 0 deletions manager/container.go
Expand Up @@ -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 {
Expand Down
48 changes: 19 additions & 29 deletions manager/manager.go
Expand Up @@ -746,6 +746,9 @@ 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()

handler, accept, err := container.NewContainerHandler(containerName, m.inHostNamespace)
if err != nil {
return err
Expand Down Expand Up @@ -775,35 +778,24 @@ 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
}
namespacedName := namespacedContainerName{
Name: containerName,
}

return false
}()
if alreadyExists {
// Check that the container didn't already exist.
if _, ok := m.containers[namespacedName]; ok {
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()
Expand All @@ -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 {
Expand Down

0 comments on commit 09f948c

Please sign in to comment.