From 559b5ef28fae9605fdaa1c093c8f35b5ebb5b4b7 Mon Sep 17 00:00:00 2001 From: Mark Rushakoff Date: Wed, 23 Oct 2024 10:11:30 -0400 Subject: [PATCH 1/2] fix(server/v2): respect context cancellation in start command When running external tests that involve starting a v2 server, the only way to get the command to shut down was through sending an interrupt or termination signal, which is not possible to do independently through many concurrent serer commands within the same process. Respect the root context being cancelled, as a signal that the server needs to be stopped. Unfortunately there are no existing unit tests in the server/v2 package with running a start command, that can be expanded to cover this behavior. --- server/v2/commands.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/server/v2/commands.go b/server/v2/commands.go index d5c202ade9f7..93286923ae17 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -120,9 +120,15 @@ func createStartCommand[T transaction.Tx]( go func() { sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - sig := <-sigCh - cancelFn() - cmd.Printf("caught %s signal\n", sig.String()) + select { + case sig := <-sigCh: + cancelFn() + cmd.Printf("caught %s signal\n", sig.String()) + case <-ctx.Done(): + // If the root context is cancelled (which is likely to happen in tests involving cobra commands), + // don't block waiting for the OS signal before stopping the server. + cancelFn() + } if err := server.Stop(ctx); err != nil { cmd.PrintErrln("failed to stop servers:", err) From a8fa60cd9360db8abc33ad5215908a12a6327e18 Mon Sep 17 00:00:00 2001 From: Mark Rushakoff Date: Wed, 23 Oct 2024 14:44:47 -0400 Subject: [PATCH 2/2] chore: change spelling in comment for linter --- server/v2/commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/v2/commands.go b/server/v2/commands.go index 93286923ae17..c64fbc1f0de4 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -125,7 +125,7 @@ func createStartCommand[T transaction.Tx]( cancelFn() cmd.Printf("caught %s signal\n", sig.String()) case <-ctx.Done(): - // If the root context is cancelled (which is likely to happen in tests involving cobra commands), + // If the root context is canceled (which is likely to happen in tests involving cobra commands), // don't block waiting for the OS signal before stopping the server. cancelFn() }