From 55b664046c446792191ef7b0be2a42e63a336be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Thu, 14 Sep 2023 11:06:46 +0200 Subject: [PATCH] libcontainer/windows: Fix process not being killed after stdio attach failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Error check in defer block used wrong error variable which is always nil if the flow reaches the defer. This caused the `newProcess.Kill` to be never called if the subsequent attemp to attach to the stdio failed. Although this only happens in Exec (as Start does overwrite the error), this also adjusts the Start to also use the returned error to avoid this kind of mistake in future changes. Signed-off-by: Paweł Gronowski --- libcontainerd/local/local_windows.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libcontainerd/local/local_windows.go b/libcontainerd/local/local_windows.go index 6cb577f4ba..0e0bb4f293 100644 --- a/libcontainerd/local/local_windows.go +++ b/libcontainerd/local/local_windows.go @@ -389,7 +389,7 @@ func (c *client) extractResourcesFromSpec(spec *specs.Spec, configuration *hcssh } } -func (ctr *container) Start(_ context.Context, _ string, withStdin bool, attachStdio libcontainerdtypes.StdioCallback) (libcontainerdtypes.Task, error) { +func (ctr *container) Start(_ context.Context, _ string, withStdin bool, attachStdio libcontainerdtypes.StdioCallback) (_ libcontainerdtypes.Task, retErr error) { ctr.mu.Lock() defer ctr.mu.Unlock() @@ -446,7 +446,7 @@ func (ctr *container) Start(_ context.Context, _ string, withStdin bool, attachS } defer func() { - if err != nil { + if retErr != nil { if err := newProcess.Kill(); err != nil { logger.WithError(err).Error("failed to kill process") } @@ -557,7 +557,7 @@ func newIOFromProcess(newProcess hcsshim.Process, terminal bool) (*cio.DirectIO, // The processID argument is entirely informational. As there is no mechanism // (exposed through the libcontainerd interfaces) to enumerate or reference an // exec'd process by ID, uniqueness is not currently enforced. -func (t *task) Exec(ctx context.Context, processID string, spec *specs.Process, withStdin bool, attachStdio libcontainerdtypes.StdioCallback) (libcontainerdtypes.Process, error) { +func (t *task) Exec(ctx context.Context, processID string, spec *specs.Process, withStdin bool, attachStdio libcontainerdtypes.StdioCallback) (_ libcontainerdtypes.Process, retErr error) { hcsContainer, err := t.getHCSContainer() if err != nil { return nil, err @@ -610,7 +610,7 @@ func (t *task) Exec(ctx context.Context, processID string, spec *specs.Process, } pid := newProcess.Pid() defer func() { - if err != nil { + if retErr != nil { if err := newProcess.Kill(); err != nil { logger.WithError(err).Error("failed to kill process") }