diff options
author | Andrei Vagin <avagin@gmail.com> | 2021-06-22 10:47:37 -0700 |
---|---|---|
committer | Andrei Vagin <avagin@gmail.com> | 2021-06-22 11:01:31 -0700 |
commit | d703340bc04a4269f420fdf24d946abcbc6a620b (patch) | |
tree | 1fbcb33795a009c56a16823611cf471d733c4aa5 /runsc | |
parent | bc27a991851fdffa59f028eecfc22bdd17ccaa55 (diff) |
runsc: don't kill sandbox, let it stop properly
The typical sequence of calls to start a container looks like this
ct, err := container.New(conf, containerArgs)
defer ct.Destroy()
ct.Start(conf)
ws, err := ct.Wait()
For the root container, ct.Destroy() kills the sandbox process. This
doesn't look like a right wait to stop it. For example, all ongoing rpc
calls are aborted in this case. If everything is going alright, we can
just wait and it will exit itself.
Reported-by: syzbot+084fca334720887441e7@syzkaller.appspotmail.com
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Diffstat (limited to 'runsc')
-rw-r--r-- | runsc/boot/controller.go | 5 | ||||
-rw-r--r-- | runsc/boot/loader.go | 7 | ||||
-rw-r--r-- | runsc/sandbox/sandbox.go | 6 |
3 files changed, 15 insertions, 3 deletions
diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index d52cf5a00..b605aa40a 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -15,6 +15,7 @@ package boot import ( + "context" "errors" "fmt" "os" @@ -165,8 +166,8 @@ func newController(fd int, l *Loader) (*controller, error) { return ctrl, nil } -func (c *controller) stop() { - c.srv.Stop() +func (c *controller) stop(ctx context.Context) { + c.srv.Stop(ctx) } // containerManager manages sandbox containers. diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 8d71d7447..d8282d1d1 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -16,6 +16,7 @@ package boot import ( + gcontext "context" "errors" "fmt" mrand "math/rand" @@ -468,6 +469,8 @@ func createProcessArgs(id string, spec *specs.Spec, creds *auth.Credentials, k * return procArgs, nil } +const destroyTimeout = 15 * gtime.Second + // Destroy cleans up all resources used by the loader. // // Note that this will block until all open control server connections have @@ -482,7 +485,9 @@ func (l *Loader) Destroy() { // Stop the control server. This will indirectly stop any // long-running control operations that are in flight, e.g. // profiling operations. - l.ctrl.stop() + ctx, cancel := gcontext.WithTimeout(context.Background(), destroyTimeout) + defer cancel() + l.ctrl.stop(ctx) // Release all kernel resources. This is only safe after we can no longer // save/restore. diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 29e202b7d..f14cc7229 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -797,7 +797,13 @@ func (s *Sandbox) Wait(cid string) (unix.WaitStatus, error) { // Try the Wait RPC to the sandbox. var ws unix.WaitStatus err = conn.Call(boot.ContainerWait, &cid, &ws) + conn.Close() if err == nil { + if s.IsRootContainer(cid) { + if err := s.waitForStopped(); err != nil { + return unix.WaitStatus(0), err + } + } // It worked! return ws, nil } |