Merge pull request #2624 from Ace-Tang/fix_delete_lock

estesp · web-flow · commit ed2bf6dd8a92 · 2018-09-11T10:26:32.000-04:00
fix: modify lock location of exec delete avoid exec hang
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`