Skip to content

[release/1.6] fix: shimv1 leak issue#9345

Merged
kzys merged 1 commit intocontainerd:release/1.6from
fuweid:16-fix-9309
Nov 10, 2023
Merged

[release/1.6] fix: shimv1 leak issue#9345
kzys merged 1 commit intocontainerd:release/1.6from
fuweid:16-fix-9309

Conversation

@fuweid
Copy link
Copy Markdown
Member

@fuweid fuweid commented Nov 7, 2023

// Delete the initial process and container
func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteResponse, error) {
        p, err := s.getInitProcess()
        if err != nil {
                return nil, err
        }
        if err := p.Delete(ctx); err != nil {
                return nil, errdefs.ToGRPC(err)
        }

	// The client might canceled the request but the shim service still
	// moved on. The `delete(s.processes, s.id)` was executed
	// successfully. So the next Delete call will return `container
	// must be created` error. The client side should ignore this
	// issue.

        s.mu.Lock()
        delete(s.processes, s.id)
        s.mu.Unlock()
        s.platform.Close()
        return &shimapi.DeleteResponse{
                ExitStatus: uint32(p.ExitStatus()),
                ExitedAt:   protobuf.ToTimestamp(p.ExitedAt()),
                Pid:        uint32(p.Pid()),
        }, nil
}

introduced by #9004
fixes: #9309

(cherry picked from commit 4499128)

```go
// Delete the initial process and container
func (s *Service) Delete(ctx context.Context, r *ptypes.Empty) (*shimapi.DeleteResponse, error) {
        p, err := s.getInitProcess()
        if err != nil {
                return nil, err
        }
        if err := p.Delete(ctx); err != nil {
                return nil, errdefs.ToGRPC(err)
        }

	// The client might canceled the request but the shim service still
	// moved on. The `delete(s.processes, s.id)` was executed
	// successfully. So the next Delete call will return `container
	// must be created` error. The client side should ignore this
	// issue.

        s.mu.Lock()
        delete(s.processes, s.id)
        s.mu.Unlock()
        s.platform.Close()
        return &shimapi.DeleteResponse{
                ExitStatus: uint32(p.ExitStatus()),
                ExitedAt:   protobuf.ToTimestamp(p.ExitedAt()),
                Pid:        uint32(p.Pid()),
        }, nil
}
```

introduced by containerd#9004
fixes: containerd#9309

Signed-off-by: Wei Fu <[email protected]>
(cherry picked from commit 4499128)
Signed-off-by: Wei Fu <[email protected]>
@johannesfrey
Copy link
Copy Markdown
Contributor

Thanks @fuweid. lgtm.

@fuweid
Copy link
Copy Markdown
Member Author

fuweid commented Nov 9, 2023

ping @mikebrow @kiashok @henry118 @estesp ~ easy to review this one. 😁 thanks

@kzys kzys merged commit 23b02b8 into containerd:release/1.6 Nov 10, 2023
@fuweid fuweid deleted the 16-fix-9309 branch November 14, 2023 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants