fix: modify lock location of exec delete

Ace-Tang · Ace-Tang · commit 079292e3fc20 · 2018-09-11T13:22:59.000+08:00
func (e *execProcess) delete(ctx context.Context) error {
    e.wg.Wait()
...
}
delete exec process will wait for io copy finish, if wait here,
other process can not get lock of shim service.

1. apply lock around s.transition() calls in the Delete methods.
2. put lock after wait io copy in exec Delete.

Signed-off-by: Ace-Tang &lt;aceapril@126.com&gt;
diff --git a/runtime/v1/linux/proc/exec_state.go b/runtime/v1/linux/proc/exec_state.go
@@ -60,11 +60,11 @@ func (s *execCreatedState) Start(ctx context.Context) error {
 }
 
 func (s *execCreatedState) Delete(ctx context.Context) error {
-	s.p.mu.Lock()
-	defer s.p.mu.Unlock()
 	if err := s.p.delete(ctx); err != nil {
 		return err
 	}
+	s.p.mu.Lock()
+	defer s.p.mu.Unlock()
 	return s.transition("deleted")
 }
 
@@ -168,11 +168,11 @@ func (s *execStoppedState) Start(ctx context.Context) error {
 }
 
 func (s *execStoppedState) Delete(ctx context.Context) error {
-	s.p.mu.Lock()
-	defer s.p.mu.Unlock()
 	if err := s.p.delete(ctx); err != nil {
 		return err
 	}
+	s.p.mu.Lock()
+	defer s.p.mu.Unlock()
 	return s.transition("deleted")
 }
 
diff --git a/runtime/v1/shim/service.go b/runtime/v1/shim/service.go
@@ -224,19 +224,21 @@ func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteR
 
 // DeleteProcess deletes an exec'd process
 func (s *Service) DeleteProcess(ctx context.Context, r *shimapi.DeleteProcessRequest) (*shimapi.DeleteResponse, error) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
 	if r.ID == s.id {
 		return nil, status.Errorf(codes.InvalidArgument, "cannot delete init process with DeleteProcess")
 	}
+	s.mu.Lock()
 	p := s.processes[r.ID]
+	s.mu.Unlock()
 	if p == nil {
 		return nil, errors.Wrapf(errdefs.ErrNotFound, "process %s", r.ID)
 	}
 	if err := p.Delete(ctx); err != nil {
 		return nil, err
 	}
+	s.mu.Lock()
 	delete(s.processes, r.ID)
+	s.mu.Unlock()
 	return &shimapi.DeleteResponse{
 		ExitStatus: uint32(p.ExitStatus()),
 		ExitedAt:   p.ExitedAt(),

Original file line number	Diff line number	Diff line change
`@@ -60,11 +60,11 @@ func (s *execCreatedState) Start(ctx context.Context) error {`
`60`	`60`	`}`
`61`	`61`
`62`	`62`	`func (s *execCreatedState) Delete(ctx context.Context) error {`
`63`		`- s.p.mu.Lock()`
`64`		`- defer s.p.mu.Unlock()`
`65`	`63`	`if err := s.p.delete(ctx); err != nil {`
`66`	`64`	`return err`
`67`	`65`	`}`
	`66`	`+ s.p.mu.Lock()`
	`67`	`+ defer s.p.mu.Unlock()`
`68`	`68`	`return s.transition("deleted")`
`69`	`69`	`}`
`70`	`70`
`@@ -168,11 +168,11 @@ func (s *execStoppedState) Start(ctx context.Context) error {`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`func (s *execStoppedState) Delete(ctx context.Context) error {`
`171`		`- s.p.mu.Lock()`
`172`		`- defer s.p.mu.Unlock()`
`173`	`171`	`if err := s.p.delete(ctx); err != nil {`
`174`	`172`	`return err`
`175`	`173`	`}`
	`174`	`+ s.p.mu.Lock()`
	`175`	`+ defer s.p.mu.Unlock()`
`176`	`176`	`return s.transition("deleted")`
`177`	`177`	`}`
`178`	`178`