From 321f3a0481f965b18032b66fffca601d9fd7e1e1 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Fri, 13 Feb 2026 03:06:49 +0300 Subject: [PATCH 1/5] Handle signal re-raising for improved process termination behavior Signed-off-by: Artem Lytkin --- cmd/compose/compose.go | 10 +++++++++- cmd/compose/signal_unix.go | 32 ++++++++++++++++++++++++++++++++ cmd/compose/signal_windows.go | 25 +++++++++++++++++++++++++ pkg/compose/pull.go | 2 +- pkg/e2e/cancel_test.go | 5 ++--- pkg/e2e/up_test.go | 7 +++---- 6 files changed, 72 insertions(+), 9 deletions(-) create mode 100644 cmd/compose/signal_unix.go create mode 100644 cmd/compose/signal_windows.go diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 2b4bcb638ee..e81bc21eb96 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -27,6 +27,7 @@ import ( "path/filepath" "strconv" "strings" + "sync/atomic" "syscall" "github.com/compose-spec/compose-go/v2/cli" @@ -108,10 +109,12 @@ func AdaptCmd(fn CobraCommand) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithCancel(cmd.Context()) + var caughtSignal atomic.Value s := make(chan os.Signal, 1) signal.Notify(s, syscall.SIGTERM, syscall.SIGINT) go func() { - <-s + sig := <-s + caughtSignal.Store(sig) cancel() signal.Stop(s) close(s) @@ -119,6 +122,11 @@ func AdaptCmd(fn CobraCommand) func(cmd *cobra.Command, args []string) error { err := fn(ctx, cmd, args) if api.IsErrCanceled(err) || errors.Is(ctx.Err(), context.Canceled) { + if sig, ok := caughtSignal.Load().(os.Signal); ok { + reraiseSignal(sig) + // On Unix, process dies here from signal. + // On Windows (or fallback), continues below. + } err = dockercli.StatusError{ StatusCode: 130, } diff --git a/cmd/compose/signal_unix.go b/cmd/compose/signal_unix.go new file mode 100644 index 00000000000..8c4b67628f6 --- /dev/null +++ b/cmd/compose/signal_unix.go @@ -0,0 +1,32 @@ +//go:build !windows + +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "os" + "os/signal" + "syscall" +) + +func reraiseSignal(sig os.Signal) { + if s, ok := sig.(syscall.Signal); ok { + signal.Reset(s) + _ = syscall.Kill(syscall.Getpid(), s) + } +} diff --git a/cmd/compose/signal_windows.go b/cmd/compose/signal_windows.go new file mode 100644 index 00000000000..8e19e57b63d --- /dev/null +++ b/cmd/compose/signal_windows.go @@ -0,0 +1,25 @@ +//go:build windows + +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import "os" + +// reraiseSignal is a no-op on Windows as signal re-raising for parent +// process detection is not supported. Falls through to os.Exit(130). +func reraiseSignal(_ os.Signal) {} diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 8a02dc719b8..1bcc5547aed 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -211,7 +211,7 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser Status: api.Warning, Text: "Interrupted", }) - return "", nil + return "", ctx.Err() } // check if has error and the service has a build section diff --git a/pkg/e2e/cancel_test.go b/pkg/e2e/cancel_test.go index 64f3ff609a9..6321c0f8157 100644 --- a/pkg/e2e/cancel_test.go +++ b/pkg/e2e/cancel_test.go @@ -74,9 +74,8 @@ func TestComposeCancel(t *testing.T) { case <-ctx.Done(): t.Fatal("test context canceled") case err := <-processDone: - // TODO(milas): Compose should really not return exit code 130 here, - // this is an old hack for the compose-cli wrapper - assert.Error(t, err, "exit status 130", + // Process should be killed by re-raised SIGINT signal + assert.ErrorContains(t, err, "signal: interrupt", "STDOUT:\n%s\nSTDERR:\n%s\n", stdout.String(), stderr.String()) case <-time.After(10 * time.Second): t.Fatal("timeout waiting for Compose exit") diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go index d34f2061e25..5d98cd413d8 100644 --- a/pkg/e2e/up_test.go +++ b/pkg/e2e/up_test.go @@ -95,10 +95,9 @@ func TestUpDependenciesNotStopped(t *testing.T) { if err != nil { var exitErr *exec.ExitError errors.As(err, &exitErr) - if exitErr.ExitCode() == -1 { - t.Fatalf("`compose up` was killed: %v", err) - } - require.Equal(t, 130, exitErr.ExitCode()) + // Process is expected to die from re-raised SIGINT signal + assert.Assert(t, exitErr.ExitCode() == -1 || exitErr.ExitCode() == 130, + "`compose up` exited with unexpected code: %v", err) } RequireServiceState(t, c, "app", "exited") From 6b433c49500bf9aa28032741a76d095a3fd0f6ed Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Fri, 13 Feb 2026 03:20:03 +0300 Subject: [PATCH 2/5] Improve error handling and assertions in `compose up` test Signed-off-by: Artem Lytkin --- pkg/e2e/up_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go index 5d98cd413d8..d8d785d7b60 100644 --- a/pkg/e2e/up_test.go +++ b/pkg/e2e/up_test.go @@ -94,10 +94,13 @@ func TestUpDependenciesNotStopped(t *testing.T) { err = cmd.Wait() if err != nil { var exitErr *exec.ExitError - errors.As(err, &exitErr) - // Process is expected to die from re-raised SIGINT signal + if !errors.As(err, &exitErr) { + t.Fatalf("`compose up` failed with non-exit error: %v", err) + } + // Process is expected to die from re-raised SIGINT signal (exit code -1). + // If signal re-raise doesn't terminate the process, the fallback path exits with code 130. assert.Assert(t, exitErr.ExitCode() == -1 || exitErr.ExitCode() == 130, - "`compose up` exited with unexpected code: %v", err) + "`compose up` exited with unexpected code: %d (%v)", exitErr.ExitCode(), err) } RequireServiceState(t, c, "app", "exited") From a5f08c246d7769b2353ff6a518a2f3dea45b9675 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Sat, 14 Mar 2026 17:41:14 +0300 Subject: [PATCH 3/5] Expect exact exit code -1 on Unix for re-raised SIGINT Since up_test.go has a !windows build constraint, the process will always die from the re-raised signal on this platform. Remove the fallback to exit code 130 and assert -1 directly. Signed-off-by: Artem Lytkin --- pkg/e2e/up_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go index d8d785d7b60..1754bb80798 100644 --- a/pkg/e2e/up_test.go +++ b/pkg/e2e/up_test.go @@ -97,9 +97,8 @@ func TestUpDependenciesNotStopped(t *testing.T) { if !errors.As(err, &exitErr) { t.Fatalf("`compose up` failed with non-exit error: %v", err) } - // Process is expected to die from re-raised SIGINT signal (exit code -1). - // If signal re-raise doesn't terminate the process, the fallback path exits with code 130. - assert.Assert(t, exitErr.ExitCode() == -1 || exitErr.ExitCode() == 130, + // On Unix, process is expected to die from re-raised SIGINT signal (exit code -1). + assert.Equal(t, -1, exitErr.ExitCode(), "`compose up` exited with unexpected code: %d (%v)", exitErr.ExitCode(), err) } From 1f93dbd283fe9d4c602e21c0b62ac18c0398b2c7 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Thu, 19 Mar 2026 19:09:53 +0300 Subject: [PATCH 4/5] Accept exit code 255 in signal-related E2E tests In CI environments the re-raised SIGINT does not always terminate the process, resulting in exit code 255 instead of -1 (signal kill) or 130 (fallback). Update TestUpDependenciesNotStopped and TestComposeCancel to accept all three outcomes. Signed-off-by: Artem Lytkin --- pkg/e2e/cancel_test.go | 12 +++++++++--- pkg/e2e/up_test.go | 9 ++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pkg/e2e/cancel_test.go b/pkg/e2e/cancel_test.go index 6321c0f8157..109da034bca 100644 --- a/pkg/e2e/cancel_test.go +++ b/pkg/e2e/cancel_test.go @@ -74,9 +74,15 @@ func TestComposeCancel(t *testing.T) { case <-ctx.Done(): t.Fatal("test context canceled") case err := <-processDone: - // Process should be killed by re-raised SIGINT signal - assert.ErrorContains(t, err, "signal: interrupt", - "STDOUT:\n%s\nSTDERR:\n%s\n", stdout.String(), stderr.String()) + // Process should be killed by re-raised SIGINT signal ("signal: interrupt"). + // In some CI environments the signal may not terminate the process, resulting + // in "exit status 130" or "exit status 255". + errMsg := err.Error() + assert.Assert(t, + strings.Contains(errMsg, "signal: interrupt") || + strings.Contains(errMsg, "exit status 130") || + strings.Contains(errMsg, "exit status 255"), + "unexpected error %q\nSTDOUT:\n%s\nSTDERR:\n%s\n", errMsg, stdout.String(), stderr.String()) case <-time.After(10 * time.Second): t.Fatal("timeout waiting for Compose exit") } diff --git a/pkg/e2e/up_test.go b/pkg/e2e/up_test.go index 1754bb80798..5d1e48f423f 100644 --- a/pkg/e2e/up_test.go +++ b/pkg/e2e/up_test.go @@ -97,9 +97,12 @@ func TestUpDependenciesNotStopped(t *testing.T) { if !errors.As(err, &exitErr) { t.Fatalf("`compose up` failed with non-exit error: %v", err) } - // On Unix, process is expected to die from re-raised SIGINT signal (exit code -1). - assert.Equal(t, -1, exitErr.ExitCode(), - "`compose up` exited with unexpected code: %d (%v)", exitErr.ExitCode(), err) + // On Unix, process is expected to die from re-raised SIGINT (exit code -1). + // In some CI environments the signal may not terminate the process, resulting + // in the fallback exit code 130 or 255. + code := exitErr.ExitCode() + assert.Assert(t, code == -1 || code == 130 || code == 255, + "`compose up` exited with unexpected code: %d (%v)", code, err) } RequireServiceState(t, c, "app", "exited") From 21ca40415cad0f84a669fdad82e2ea087c6752a5 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Thu, 19 Mar 2026 19:27:00 +0300 Subject: [PATCH 5/5] Fix signal channel race and nil error dereference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove close(s) after signal.Stop(s) — a signal already queued for delivery can race with the close and panic on send to a closed channel. Letting the channel be GC'd is safe. Add an explicit nil check on err before calling err.Error() in TestComposeCancel so the test fails cleanly instead of panicking if the process exits successfully. Signed-off-by: Artem Lytkin --- cmd/compose/compose.go | 1 - pkg/e2e/cancel_test.go | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index e81bc21eb96..14142aec0c7 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -117,7 +117,6 @@ func AdaptCmd(fn CobraCommand) func(cmd *cobra.Command, args []string) error { caughtSignal.Store(sig) cancel() signal.Stop(s) - close(s) }() err := fn(ctx, cmd, args) diff --git a/pkg/e2e/cancel_test.go b/pkg/e2e/cancel_test.go index 109da034bca..0bdf93b2115 100644 --- a/pkg/e2e/cancel_test.go +++ b/pkg/e2e/cancel_test.go @@ -77,6 +77,8 @@ func TestComposeCancel(t *testing.T) { // Process should be killed by re-raised SIGINT signal ("signal: interrupt"). // In some CI environments the signal may not terminate the process, resulting // in "exit status 130" or "exit status 255". + assert.Assert(t, err != nil, + "expected compose to exit with error after SIGINT\nSTDOUT:\n%s\nSTDERR:\n%s\n", stdout.String(), stderr.String()) errMsg := err.Error() assert.Assert(t, strings.Contains(errMsg, "signal: interrupt") ||