From 0ae974e63911804d4a2007bd8a9b3ad81d6d2a90 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Thu, 6 Jan 2022 08:26:37 -0500 Subject: [PATCH] libs/os: remove trap signal (#7515) --- cmd/priv_val_server/main.go | 11 ++++-- cmd/tendermint/commands/light.go | 12 +++--- docs/app-dev/abci-cli.md | 18 ++++----- libs/os/os.go | 24 ------------ libs/os/os_test.go | 67 -------------------------------- rpc/jsonrpc/test/main.go | 8 ++-- 6 files changed, 24 insertions(+), 116 deletions(-) diff --git a/cmd/priv_val_server/main.go b/cmd/priv_val_server/main.go index 8bd67fbda..0708edd5c 100644 --- a/cmd/priv_val_server/main.go +++ b/cmd/priv_val_server/main.go @@ -9,6 +9,8 @@ import ( "net" "net/http" "os" + "os/signal" + "syscall" "time" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" @@ -19,7 +21,6 @@ import ( "github.com/tendermint/tendermint/libs/log" tmnet "github.com/tendermint/tendermint/libs/net" - tmos "github.com/tendermint/tendermint/libs/os" "github.com/tendermint/tendermint/privval" grpcprivval "github.com/tendermint/tendermint/privval/grpc" privvalproto "github.com/tendermint/tendermint/proto/tendermint/privval" @@ -133,8 +134,10 @@ func main() { os.Exit(1) } - // Stop upon receiving SIGTERM or CTRL-C. - tmos.TrapSignal(ctx, logger, func() { + opctx, opcancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) + defer opcancel() + go func() { + <-opctx.Done() logger.Debug("SignerServer: calling Close") if *prometheusAddr != "" { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) @@ -145,7 +148,7 @@ func main() { } } s.GracefulStop() - }) + }() // Run forever. select {} diff --git a/cmd/tendermint/commands/light.go b/cmd/tendermint/commands/light.go index dc466ea0e..31171174d 100644 --- a/cmd/tendermint/commands/light.go +++ b/cmd/tendermint/commands/light.go @@ -17,7 +17,6 @@ import ( "github.com/tendermint/tendermint/libs/log" tmmath "github.com/tendermint/tendermint/libs/math" - tmos "github.com/tendermint/tendermint/libs/os" "github.com/tendermint/tendermint/light" lproxy "github.com/tendermint/tendermint/light/proxy" lrpc "github.com/tendermint/tendermint/light/rpc" @@ -188,15 +187,14 @@ func runProxy(cmd *cobra.Command, args []string) error { return err } - // Stop upon receiving SIGTERM or CTRL-C. - tmos.TrapSignal(cmd.Context(), logger, func() { - p.Listener.Close() - }) - - // this might be redundant to the above, eventually. ctx, cancel := signal.NotifyContext(cmd.Context(), syscall.SIGTERM) defer cancel() + go func() { + <-ctx.Done() + p.Listener.Close() + }() + logger.Info("Starting proxy...", "laddr", listenAddr) if err := p.ListenAndServe(ctx); err != http.ErrServerClosed { // Error starting or closing listener: diff --git a/docs/app-dev/abci-cli.md b/docs/app-dev/abci-cli.md index cc0afdc55..27b58721b 100644 --- a/docs/app-dev/abci-cli.md +++ b/docs/app-dev/abci-cli.md @@ -83,19 +83,19 @@ func cmdKVStore(cmd *cobra.Command, args []string) error { if err != nil { return err } + + // Stop upon receiving SIGTERM or CTRL-C. + ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer cancel() + srv.SetLogger(logger.With("module", "abci-server")) - if err := srv.Start(); err != nil { + if err := srv.Start(ctx); err != nil { return err } - // Stop upon receiving SIGTERM or CTRL-C. - tmos.TrapSignal(logger, func() { - // Cleanup - srv.Stop() - }) - - // Run forever. - select {} + // Run until shutdown. +<-ctx.Done() +srv.Wait() } ``` diff --git a/libs/os/os.go b/libs/os/os.go index 0fd50da5b..3d74c2208 100644 --- a/libs/os/os.go +++ b/libs/os/os.go @@ -1,36 +1,12 @@ package os import ( - "context" "errors" "fmt" "io" "os" - "os/signal" - "syscall" ) -type logger interface { - Info(msg string, keyvals ...interface{}) -} - -// TrapSignal catches SIGTERM and SIGINT, executes the cleanup function, -// and exits with code 0. -func TrapSignal(ctx context.Context, logger logger, cb func()) { - opctx, opcancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) - - go func() { - defer opcancel() - defer opcancel() - <-opctx.Done() - logger.Info("captured signal, exiting...") - if cb != nil { - cb() - } - os.Exit(0) - }() -} - // EnsureDir ensures the given directory exists, creating it if necessary. // Errors if the path already exists as a non-directory. func EnsureDir(dir string, mode os.FileMode) error { diff --git a/libs/os/os_test.go b/libs/os/os_test.go index eec3e723b..fe503f921 100644 --- a/libs/os/os_test.go +++ b/libs/os/os_test.go @@ -2,14 +2,10 @@ package os_test import ( "bytes" - "context" "fmt" "os" - "os/exec" "path/filepath" - "syscall" "testing" - "time" "github.com/stretchr/testify/require" tmos "github.com/tendermint/tendermint/libs/os" @@ -43,32 +39,6 @@ func TestCopyFile(t *testing.T) { os.Remove(copyfile) } -func TestTrapSignal(t *testing.T) { - if os.Getenv("TM_TRAP_SIGNAL_TEST") == "1" { - t.Log("inside test process") - killer() - return - } - - cmd, _, mockStderr := newTestProgram(t, "TM_TRAP_SIGNAL_TEST") - - err := cmd.Run() - if err == nil { - wantStderr := "exiting" - if mockStderr.String() != wantStderr { - t.Fatalf("stderr: want %q, got %q", wantStderr, mockStderr.String()) - } - - return - } - - if e, ok := err.(*exec.ExitError); ok && !e.Success() { - t.Fatalf("wrong exit code, want 0, got %d", e.ExitCode()) - } - - t.Fatal("this error should not be triggered") -} - func TestEnsureDir(t *testing.T) { tmp, err := os.MkdirTemp("", "ensure-dir") require.NoError(t, err) @@ -102,43 +72,6 @@ func TestEnsureDir(t *testing.T) { require.Error(t, err) } -type mockLogger struct{} - -func (ml mockLogger) Info(msg string, keyvals ...interface{}) {} - -func killer() { - logger := mockLogger{} - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - tmos.TrapSignal(ctx, logger, func() { _, _ = fmt.Fprintf(os.Stderr, "exiting") }) - time.Sleep(1 * time.Second) - - p, err := os.FindProcess(os.Getpid()) - if err != nil { - panic(err) - } - - if err := p.Signal(syscall.SIGTERM); err != nil { - panic(err) - } - - time.Sleep(1 * time.Second) -} - -func newTestProgram(t *testing.T, environVar string) (cmd *exec.Cmd, stdout *bytes.Buffer, stderr *bytes.Buffer) { - t.Helper() - - cmd = exec.Command(os.Args[0], "-test.run="+t.Name()) - stdout, stderr = bytes.NewBufferString(""), bytes.NewBufferString("") - cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", environVar)) - cmd.Stdout = stdout - cmd.Stderr = stderr - - return -} - // Ensure that using CopyFile does not truncate the destination file before // the origin is positively a non-directory and that it is ready for copying. // See https://github.com/tendermint/tendermint/issues/6427 diff --git a/rpc/jsonrpc/test/main.go b/rpc/jsonrpc/test/main.go index 8aadc3ec6..f11709f98 100644 --- a/rpc/jsonrpc/test/main.go +++ b/rpc/jsonrpc/test/main.go @@ -5,9 +5,10 @@ import ( "fmt" "net/http" "os" + "os/signal" + "syscall" "github.com/tendermint/tendermint/libs/log" - tmos "github.com/tendermint/tendermint/libs/os" rpcserver "github.com/tendermint/tendermint/rpc/jsonrpc/server" rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types" ) @@ -30,12 +31,9 @@ func main() { logger = log.MustNewDefaultLogger(log.LogFormatPlain, log.LogLevelInfo, false) ) - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() - // Stop upon receiving SIGTERM or CTRL-C. - tmos.TrapSignal(ctx, logger, func() {}) - rpcserver.RegisterRPCFuncs(mux, routes, logger) config := rpcserver.DefaultConfig() listener, err := rpcserver.Listen("tcp://127.0.0.1:8008", config.MaxOpenConnections)