github/containerd
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

runtime/v2/runc: fix leaking socket path

Browse Source 
When runC shimv2 starts, the StartShim interface will re-exec itself as
long-running process, which will read the `address` during initializing.

```happycase
Process

containerd-shim-runc-v1/v2 start             containerd-shim-runc-v1/v2

	initializing socket

	reexec containerd-shim-runc-v1/v2

	write address into file

						initializing

							read address

	write back to containerd daemon

						serving

						...

						remove address in Shutdown call
```

However, there is no synchronization after reexec. Then the data race is
like:

```leaking-case
Process

containerd-shim-runc-v1/v2 start             containerd-shim-runc-v1/v2

	initializing socket

	reexec containerd-shim-runc-v1/v2

						initializing

							read address

	write address into file

	write back to containerd daemon

						serving

						...

						fail to remove address
						because of empty address
```

The `address` should be writen into file first before reexec.

And if shutdown the whole service before cleanup temporary
resource (like socket file), the Shutdown caller will receive `ttrpc: closed`
sometime, which depends on go runtime scheduler. Then it also causes leaking
socket files.

Since the shimV2-Delete binary API must be called to cleanup shim temporary
resource and shimV2-runC-v1 doesn't support grouping multi containers in one,
it is safe to remove the socket file in the binary call for shimV2-runC-v1.
But for the shimV2-runC-v2 shim, we still cleanup socket in Shutdown.
Hopefully we can find a way to cleanup socket in shimV2-Delete binary
call.

Fix: #5173

Signed-off-by: Wei Fu <fuweid89@gmail.com>
This commit is contained in:
Wei Fu
2021-03-15 12:30:06 +08:00
parent d8208e2e37
commit d895118c7c
3 changed files with 34 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
runtime/v2/runc/v1/service.go
View File

@@ -153,6 +153,11 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
_ = shim.RemoveSocket(address)
}
}()
// make sure that reexec shim-v2 binary use the value if need
if err := shim.WriteAddress("address", address); err != nil {
return "", err
}
f, err := socket.File()
if err != nil {
return "", err
@@ -174,9 +179,6 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
if err := shim.WritePidFile("shim.pid", cmd.Process.Pid); err != nil {
return "", err
}
if err := shim.WriteAddress("address", address); err != nil {
return "", err
}
if data, err := ioutil.ReadAll(os.Stdin); err == nil {
if len(data) > 0 {
var any ptypes.Any
@@ -209,6 +211,12 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
}
func (s *service) Cleanup(ctx context.Context) (*taskAPI.DeleteResponse, error) {
if address, err := shim.ReadAddress("address"); err == nil {
if err = shim.RemoveSocket(address); err != nil {
return nil, err
}
}
path, err := os.Getwd()
if err != nil {
return nil, err
@@ -562,11 +570,10 @@ func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (*task
}
func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*ptypes.Empty, error) {
// please make sure that temporary resource has been cleanup
// before shutdown service.
s.cancel()
close(s.events)
if address, err := shim.ReadAddress("address"); err == nil {
_ = shim.RemoveSocket(address)
}
return empty, nil
}