diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 09146ce7d..b76336fb6 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -488,6 +488,10 @@ type RestoreOpts struct { func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { log.Debugf("containerManager.Restore") + cm.l.mu.Lock() + cu := cleanup.Make(cm.l.mu.Unlock) + defer cu.Clean() + if cm.l.state == restoring { return fmt.Errorf("restore is already in progress") } @@ -514,6 +518,8 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { cm.restorer = &restorer{restoreDone: cm.onRestoreDone, stateFile: stateFile} cm.l.restoreWaiters = sync.NewCond(&cm.l.mu) cm.l.state = restoring + // Release `cm.l.mu`. + cu.Clean() fileIdx := 1 if o.HavePagesFile { @@ -590,9 +596,12 @@ func (cm *containerManager) onRestoreDone() error { func (cm *containerManager) RestoreSubcontainer(args *StartArgs, _ *struct{}) error { log.Debugf("containerManager.RestoreSubcontainer, cid: %s, args: %+v", args.CID, args) + cm.l.mu.Lock() if cm.l.state != restoring { + cm.l.mu.Unlock() return fmt.Errorf("sandbox is not being restored, cannot restore subcontainer") } + cm.l.mu.Unlock() // Validate arguments. if args.Spec == nil { @@ -808,8 +817,9 @@ func (cm *containerManager) ProcfsDump(_ *struct{}, out *[]procfs.ProcessProcfsD log.Debugf("containerManager.ProcfsDump") ts := cm.l.k.TaskSet() pidns := ts.Root - *out = make([]procfs.ProcessProcfsDump, 0, len(cm.l.processes)) - for _, tg := range pidns.ThreadGroups() { + tgs := pidns.ThreadGroups() + *out = make([]procfs.ProcessProcfsDump, 0, len(tgs)) + for _, tg := range tgs { pid := pidns.IDOfThreadGroup(tg) procDump, err := procfs.Dump(tg.Leader(), pid, pidns) if err != nil { @@ -848,6 +858,8 @@ func (cm *containerManager) Mount(args *MountArgs, _ *struct{}) error { var cu cleanup.Cleanup defer cu.Clean() + cm.l.mu.Lock() + defer cm.l.mu.Unlock() eid := execID{cid: args.ContainerID} ep, ok := cm.l.processes[eid] if !ok { diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 1c04de808..94f925b8a 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -211,40 +211,41 @@ type Loader struct { // mu guards the fields below. mu sync.Mutex - // state is guarded by mu. + // +checklocks:mu state loaderState // sharedMounts holds VFS mounts that may be shared between containers within // the same pod. It is mapped by mount source. // - // sharedMounts is guarded by mu. + // +checklocks:mu sharedMounts map[string]*vfs.Mount // processes maps containers init process and invocation of exec. Root // processes are keyed with container ID and pid=0, while exec invocations // have the corresponding pid set. // - // processes is guarded by mu. + // +checklocks:mu processes map[execID]*execProcess // containerIDs store container names and IDs to assist with restore and container // naming when user didn't provide one. // // Mapping: name -> cid. - // containerIDs is guarded by mu. + // +checklocks:mu containerIDs map[string]string // containerSpecs stores container specs for each container in sandbox. // // Mapping: cid -> spec. - // containerSpecs is guarded by mu. + // +checklocks:mu containerSpecs map[string]*specs.Spec // portForwardProxies is a list of active port forwarding connections. // - // portForwardProxies is guarded by mu. + // +checklocks:mu portForwardProxies []*pf.Proxy + // +checklocks:mu saveFDs []*fd.FD } @@ -429,7 +430,7 @@ func New(args Args) (*Loader, error) { saveFDs: args.SaveFDs, } - containerName := l.registerContainerLocked(args.Spec, args.ID) + containerName := l.registerContainer(args.Spec, args.ID) l.root = containerInfo{ cid: args.ID, containerName: containerName, @@ -705,9 +706,11 @@ func (l *Loader) Destroy() { l.watchdog.Stop() ctx := l.k.SupervisorContext() + l.mu.Lock() for _, m := range l.sharedMounts { m.DecRef(ctx) } + l.mu.Unlock() // Stop the control server. This will indirectly stop any // long-running control operations that are in flight, e.g. @@ -1673,7 +1676,8 @@ func (l *Loader) threadGroupFromID(key execID) (*kernel.ThreadGroup, error) { // tryThreadGroupFromIDLocked returns the thread group for the given execution // ID. It may return nil in case the container has not started yet. Returns // error if execution ID is invalid or if the container cannot be found (maybe -// it has been deleted). Caller must hold 'mu'. +// it has been deleted). +// +checklocks:l.mu func (l *Loader) tryThreadGroupFromIDLocked(key execID) (*kernel.ThreadGroup, error) { ep, err := l.findProcessLocked(key) if err != nil { @@ -1685,7 +1689,8 @@ func (l *Loader) tryThreadGroupFromIDLocked(key execID) (*kernel.ThreadGroup, er // ttyFromIDLocked returns the TTY files for the given execution ID. It may // return nil in case the container has not started yet. Returns error if // execution ID is invalid or if the container cannot be found (maybe it has -// been deleted). Caller must hold 'mu'. +// been deleted). +// +checklocks:l.mu func (l *Loader) ttyFromIDLocked(key execID) (*host.TTYFileDescription, error) { ep, err := l.findProcessLocked(key) if err != nil { @@ -1868,6 +1873,7 @@ func (l *Loader) networkStats() ([]*NetworkInterface, error) { return stats, nil } +// +checklocks:l.mu func (l *Loader) findProcessLocked(key execID) (*execProcess, error) { ep := l.processes[key] if ep == nil { @@ -1883,6 +1889,7 @@ func (l *Loader) registerContainer(spec *specs.Spec, cid string) string { return l.registerContainerLocked(spec, cid) } +// +checklocks:l.mu func (l *Loader) registerContainerLocked(spec *specs.Spec, cid string) string { containerName := specutils.ContainerName(spec) if len(containerName) == 0 { diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index 57f91219d..414e553c8 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -480,6 +480,8 @@ func TestCreateMountNamespace(t *testing.T) { defer l.Destroy() defer loaderCleanup() + l.mu.Lock() + defer l.mu.Unlock() mntr := newContainerMounter(&l.root, l.k, l.mountHints, l.sharedMounts, "", l.sandboxID) ctx := l.k.SupervisorContext() creds := auth.NewRootCredentials(l.root.procArgs.Credentials.UserNamespace) diff --git a/runsc/boot/restore.go b/runsc/boot/restore.go index 0343f2c74..a1607c1a9 100644 --- a/runsc/boot/restore.go +++ b/runsc/boot/restore.go @@ -189,6 +189,12 @@ func (r *restorer) restore(l *Loader) error { ctx = context.WithValue(ctx, stack.CtxRestoreStack, oldStack) } + l.mu.Lock() + cu := cleanup.Make(func() { + l.mu.Unlock() + }) + defer cu.Clean() + fdmap := make(map[vfs.RestoreID]int) mfmap := make(map[string]*pgalloc.MemoryFile) for _, cont := range r.containers { @@ -233,12 +239,6 @@ func (r *restorer) restore(l *Loader) error { l.restore = true l.sandboxID = l.root.cid - l.mu.Lock() - cu := cleanup.Make(func() { - l.mu.Unlock() - }) - defer cu.Clean() - // Update all tasks in the system with their respective new container IDs. for _, task := range l.k.TaskSet().Root.Tasks() { oldCid := task.ContainerID()