Merge pull request cri-o#20 from runcom/locks

server: locking around server components

server/runtime.go

@@ -191,27 +191,32 @@ func (s *Server) StopPodSandbox(ctx context.Context, req *pb.StopPodSandboxReque
 	if *sbName == "" {
 		return nil, fmt.Errorf("PodSandboxId should not be empty")
 	}
-	sb := s.state.sandboxes[*sbName]
+	sb := s.getSandbox(*sbName)
 	if sb == nil {
 		return nil, fmt.Errorf("specified sandbox not found: %s", *sbName)
 	}
 
 	podInfraContainer := *sbName + "-infra"
+	sb.containersLock.Lock()
 	for _, c := range sb.containers {
 		if podInfraContainer == c.Name() {
 			podNamespace := ""
 			netnsPath, err := c.NetNsPath()
 			if err != nil {
+				sb.containersLock.Unlock()
 				return nil, err
 			}
 			if err := s.netPlugin.TearDownPod(netnsPath, podNamespace, *sbName, podInfraContainer); err != nil {
+				sb.containersLock.Unlock()
 				return nil, fmt.Errorf("failed to destroy network for container %s in sandbox %s: %v", c.Name(), *sbName, err)
 			}
 		}
 		if err := s.runtime.StopContainer(c); err != nil {
+			sb.containersLock.Unlock()
 			return nil, fmt.Errorf("failed to stop container %s in sandbox %s: %v", c.Name(), *sbName, err)
 		}
 	}
+	sb.containersLock.Unlock()
 
 	return &pb.StopPodSandboxResponse{}, nil
 }
@@ -223,26 +228,31 @@ func (s *Server) RemovePodSandbox(ctx context.Context, req *pb.RemovePodSandboxR
 	if *sbName == "" {
 		return nil, fmt.Errorf("PodSandboxId should not be empty")
 	}
-	sb := s.state.sandboxes[*sbName]
+	sb := s.getSandbox(*sbName)
 	if sb == nil {
 		return nil, fmt.Errorf("specified sandbox not found: %s", *sbName)
 	}
 
 	podInfraContainer := *sbName + "-infra"
 
 	// Delete all the containers in the sandbox
+	sb.containersLock.Lock()
 	for _, c := range sb.containers {
 		if err := s.runtime.DeleteContainer(c); err != nil {
+			sb.containersLock.Unlock()
 			return nil, fmt.Errorf("failed to delete container %s in sandbox %s: %v", c.Name(), *sbName, err)
 		}
 		if podInfraContainer == c.Name() {
+			sb.containersLock.Unlock()
 			continue
 		}
 		containerDir := filepath.Join(s.runtime.ContainerDir(), c.Name())
 		if err := os.RemoveAll(containerDir); err != nil {
+			sb.containersLock.Unlock()
 			return nil, fmt.Errorf("failed to remove container %s directory: %v", c.Name(), err)
 		}
 	}
+	sb.containersLock.Unlock()
 
 	// Remove the files related to the sandbox
 	podSandboxDir := filepath.Join(s.sandboxDir, *sbName)
@@ -271,13 +281,13 @@ func (s *Server) PodSandboxStatus(ctx context.Context, req *pb.PodSandboxStatusR
 	if *sbName == "" {
 		return nil, fmt.Errorf("PodSandboxId should not be empty")
 	}
-	sb := s.state.sandboxes[*sbName]
+	sb := s.getSandbox(*sbName)
 	if sb == nil {
 		return nil, fmt.Errorf("specified sandbox not found: %s", *sbName)
 	}
 
 	podInfraContainerName := *sbName + "-infra"
-	podInfraContainer := sb.containers[podInfraContainerName]
+	podInfraContainer := sb.getContainer(podInfraContainerName)
 
 	cState := s.runtime.ContainerStatus(podInfraContainer)
 	created := cState.Created.Unix()

server/server.go

@@ -3,6 +3,7 @@ package server
 import (
 	"fmt"
 	"os"
+	"sync"
 
 	"github.com/kubernetes-incubator/ocid/oci"
 	"github.com/kubernetes-incubator/ocid/utils"
@@ -18,6 +19,7 @@ const (
 type Server struct {
 	runtime    *oci.Runtime
 	sandboxDir string
+	stateLock  sync.Mutex
 	state      *serverState
 	netPlugin  ocicni.CNIPlugin
 }
@@ -63,37 +65,71 @@ type serverState struct {
 }
 
 type sandbox struct {
-	name       string
-	logDir     string
-	labels     map[string]string
-	containers map[string]*oci.Container
+	name           string
+	logDir         string
+	labels         map[string]string
+	containersLock sync.Mutex
+	containers     map[string]*oci.Container
+}
+
+func (s *sandbox) addContainer(c *oci.Container) {
+	s.containersLock.Lock()
+	s.containers[c.Name()] = c
+	s.containersLock.Unlock()
+}
+
+func (s *sandbox) getContainer(name string) *oci.Container {
+	s.containersLock.Lock()
+	c := s.containers[name]
+	s.containersLock.Unlock()
+	return c
+}
+
+func (s *sandbox) removeContainer(c *oci.Container) {
+	s.containersLock.Lock()
+	delete(s.containers, c.Name())
+	s.containersLock.Unlock()
 }
 
 func (s *Server) addSandbox(sb *sandbox) {
+	s.stateLock.Lock()
 	s.state.sandboxes[sb.name] = sb
+	s.stateLock.Unlock()
+}
+
+func (s *Server) getSandbox(name string) *sandbox {
+	s.stateLock.Lock()
+	sb := s.state.sandboxes[name]
+	s.stateLock.Unlock()
+	return sb
 }
 
 func (s *Server) hasSandbox(name string) bool {
+	s.stateLock.Lock()
 	_, ok := s.state.sandboxes[name]
+	s.stateLock.Unlock()
 	return ok
 }
 
-func (s *sandbox) addContainer(c *oci.Container) {
-	s.containers[c.Name()] = c
-}
-
-func (s *sandbox) removeContainer(c *oci.Container) {
-	delete(s.containers, c.Name())
-}
-
 func (s *Server) addContainer(c *oci.Container) {
+	s.stateLock.Lock()
 	sandbox := s.state.sandboxes[c.Sandbox()]
 	sandbox.addContainer(c)
 	s.state.containers[c.Name()] = c
+	s.stateLock.Unlock()
+}
+
+func (s *Server) getContainer(name string) *oci.Container {
+	s.stateLock.Lock()
+	c := s.state.containers[name]
+	s.stateLock.Unlock()
+	return c
 }
 
 func (s *Server) removeContainer(c *oci.Container) {
+	s.stateLock.Lock()
 	sandbox := s.state.sandboxes[c.Sandbox()]
 	sandbox.removeContainer(c)
 	delete(s.state.containers, c.Name())
+	s.stateLock.Unlock()
 }