From 7b0c78bacd477b2c2f7f1dd92e65e6d91d9a51e4 Mon Sep 17 00:00:00 2001 From: "Justin Terry (VM)" Date: Tue, 21 May 2019 13:57:42 -0700 Subject: [PATCH] ExecSync should block unless client context is canceled A call to ExecSync should only return if the client context was canceled or exceeded. The Timeout parameter to ExecSyncRequest is now used to send SIGKILL if the exec'd process does not exit within Timeout but all paths wait for the exec to exit. Signed-off-by: Justin Terry (VM) --- pkg/server/container_execsync.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/server/container_execsync.go b/pkg/server/container_execsync.go index 6116ee523..2358b9941 100644 --- a/pkg/server/container_execsync.go +++ b/pkg/server/container_execsync.go @@ -164,16 +164,15 @@ func (c *criService) execInContainer(ctx context.Context, id string, opts execOp }, }) - var timeoutCh <-chan time.Time - if opts.timeout == 0 { - // Do not set timeout if it's 0. - timeoutCh = make(chan time.Time) - } else { - timeoutCh = time.After(opts.timeout) + execCtx := ctx + if opts.timeout > 0 { + var execCtxCancel context.CancelFunc + execCtx, execCtxCancel = context.WithTimeout(ctx, opts.timeout) + defer execCtxCancel() } + select { - case <-timeoutCh: - //TODO(Abhi) Use context.WithDeadline instead of timeout. + case <-execCtx.Done(): // Ignore the not found error because the process may exit itself before killing. if err := process.Kill(ctx, syscall.SIGKILL); err != nil && !errdefs.IsNotFound(err) { return nil, errors.Wrapf(err, "failed to kill exec %q", execID) @@ -184,7 +183,7 @@ func (c *criService) execInContainer(ctx context.Context, id string, opts execOp execID, exitRes.ExitCode(), exitRes.Error()) <-attachDone logrus.Debugf("Stream pipe for exec process %q done", execID) - return nil, errors.Errorf("timeout %v exceeded", opts.timeout) + return nil, errors.Wrapf(execCtx.Err(), "timeout %v exceeded", opts.timeout) case exitRes := <-exitCh: code, _, err := exitRes.Result() logrus.Infof("Exec process %q exits with exit code %d and error %v", execID, code, err)