Skip to content

Commit

Permalink
signals/utils: always handle received signals
Browse files Browse the repository at this point in the history
The changes in
dcbf005
fixed the "cancellable context" detection, and made it so that Compose
would conditionally set up signal handling when the context was already
not cancellable/when the plugin was running through the CLI, as we'd
introduced a mechanism into the CLI to signal plugins to exit through a
socket instead of handling signals themselves.

This had some (not noticed at the time) issues when running through the
CLI as, due to sharing a process group id with the parent CLI process,
when a user CTRL-Cs the CLI will notify the plugin via the socket but
the plugin process itself will also be signalled if attached to the TTY.
This impacted some Compose commands that don't set up signal handling -
so not `compose up`, but other commands would immediately quit instead
of getting some "graceful" cancelled output.

We initially attempted to address this "double notification" issue in
the CLI by executing plugins under a new pgid so that they wouldn't be
signalled, but that posed an issue with Buildx reading from the TTY,
(see: moby/moby#47073) so we reverted the
process group id changes and ended at a temporary solution in
docker/cli#4792 where the CLI will only notify
plugins via the socket when they are not already going to be signalled
(when attached to a TTY).

Due to this, plugins should always set up some signal handling, which
this commit implements.

Signed-off-by: Laura Brehm <[email protected]>
  • Loading branch information
laurazard committed Jan 17, 2024
1 parent f414bf7 commit 898e1b6
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions cmd/compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"

"github.com/docker/cli/cli-plugins/plugin"
"github.com/docker/compose/v2/cmd/formatter"
"github.com/docker/compose/v2/pkg/api"
"github.com/docker/compose/v2/pkg/compose"
Expand Down Expand Up @@ -74,20 +73,17 @@ type CobraCommand func(context.Context, *cobra.Command, []string) error
// AdaptCmd adapt a CobraCommand func to cobra library
func AdaptCmd(fn CobraCommand) func(cmd *cobra.Command, args []string) error {
return func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
contextString := fmt.Sprintf("%s", ctx)
if !strings.Contains(contextString, ".WithCancel") || plugin.RunningStandalone() { // need to handle cancel
cancellableCtx, cancel := context.WithCancel(cmd.Context())
ctx = cancellableCtx
s := make(chan os.Signal, 1)
signal.Notify(s, syscall.SIGTERM, syscall.SIGINT)
go func() {
<-s
cancel()
signal.Stop(s)
close(s)
}()
}
ctx, cancel := context.WithCancel(cmd.Context())

s := make(chan os.Signal, 1)
signal.Notify(s, syscall.SIGTERM, syscall.SIGINT)
go func() {
<-s
cancel()
signal.Stop(s)
close(s)
}()

err := fn(ctx, cmd, args)
var composeErr compose.Error
if api.IsErrCanceled(err) || errors.Is(ctx.Err(), context.Canceled) {
Expand Down

0 comments on commit 898e1b6

Please sign in to comment.