From ea5c94cdb94f74808958732d8b3254921f52be4f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 15 Jul 2021 17:33:55 +0200 Subject: [PATCH] pkg/signal: move signal.DumpStacks() to a separate package It is not directly related to signal-handling, so can well live in its own package. Also added a variant that doesn't take a directory to write files to, for easier consumption / better match to how it's used. Signed-off-by: Sebastiaan van Stijn --- daemon/cluster/cluster.go | 4 +-- daemon/cluster/swarm.go | 4 +-- daemon/debugtrap_unix.go | 4 +-- daemon/debugtrap_windows.go | 4 +-- libnetwork/diagnostic/server.go | 4 +-- pkg/signal/signal_deprecated.go | 8 +++++ pkg/signal/trap.go | 43 ++----------------------- pkg/signal/trap_linux_test.go | 17 ---------- pkg/stack/stackdump.go | 57 +++++++++++++++++++++++++++++++++ pkg/stack/stackdump_test.go | 31 ++++++++++++++++++ 10 files changed, 108 insertions(+), 68 deletions(-) create mode 100644 pkg/signal/signal_deprecated.go create mode 100644 pkg/stack/stackdump.go create mode 100644 pkg/stack/stackdump_test.go diff --git a/daemon/cluster/cluster.go b/daemon/cluster/cluster.go index b47fad5936..38a3822362 100644 --- a/daemon/cluster/cluster.go +++ b/daemon/cluster/cluster.go @@ -54,7 +54,7 @@ import ( "github.com/docker/docker/daemon/cluster/controllers/plugin" executorpkg "github.com/docker/docker/daemon/cluster/executor" lncluster "github.com/docker/docker/libnetwork/cluster" - "github.com/docker/docker/pkg/signal" + "github.com/docker/docker/pkg/stack" swarmapi "github.com/docker/swarmkit/api" swarmnode "github.com/docker/swarmkit/node" "github.com/pkg/errors" @@ -393,7 +393,7 @@ func (c *Cluster) Cleanup() { if err := node.Stop(); err != nil { logrus.Errorf("failed to shut down cluster node: %v", err) - signal.DumpStacks("") + stack.Dump() } c.mu.Lock() diff --git a/daemon/cluster/swarm.go b/daemon/cluster/swarm.go index 30418c7f68..e7bb7cc21d 100644 --- a/daemon/cluster/swarm.go +++ b/daemon/cluster/swarm.go @@ -13,7 +13,7 @@ import ( "github.com/docker/docker/daemon/cluster/convert" "github.com/docker/docker/errdefs" "github.com/docker/docker/opts" - "github.com/docker/docker/pkg/signal" + "github.com/docker/docker/pkg/stack" swarmapi "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/manager/encryption" swarmnode "github.com/docker/swarmkit/node" @@ -399,7 +399,7 @@ func (c *Cluster) Leave(force bool) error { // release readers in here if err := nr.Stop(); err != nil { logrus.Errorf("failed to shut down cluster node: %v", err) - signal.DumpStacks("") + stack.Dump() return err } diff --git a/daemon/debugtrap_unix.go b/daemon/debugtrap_unix.go index bb0adfcc08..a29e8c2d1b 100644 --- a/daemon/debugtrap_unix.go +++ b/daemon/debugtrap_unix.go @@ -6,7 +6,7 @@ import ( "os" "os/signal" - stackdump "github.com/docker/docker/pkg/signal" + "github.com/docker/docker/pkg/stack" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -16,7 +16,7 @@ func (daemon *Daemon) setupDumpStackTrap(root string) { signal.Notify(c, unix.SIGUSR1) go func() { for range c { - path, err := stackdump.DumpStacks(root) + path, err := stack.DumpToFile(root) if err != nil { logrus.WithError(err).Error("failed to write goroutines dump") } else { diff --git a/daemon/debugtrap_windows.go b/daemon/debugtrap_windows.go index 19554dd7ae..56e505e49b 100644 --- a/daemon/debugtrap_windows.go +++ b/daemon/debugtrap_windows.go @@ -5,7 +5,7 @@ import ( "os" "unsafe" - "github.com/docker/docker/pkg/signal" + "github.com/docker/docker/pkg/stack" "github.com/sirupsen/logrus" "golang.org/x/sys/windows" ) @@ -34,7 +34,7 @@ func (daemon *Daemon) setupDumpStackTrap(root string) { logrus.Debugf("Stackdump - waiting signal at %s", event) for { windows.WaitForSingleObject(h, windows.INFINITE) - path, err := signal.DumpStacks(root) + path, err := stack.DumpToFile(root) if err != nil { logrus.WithError(err).Error("failed to write goroutines dump") } else { diff --git a/libnetwork/diagnostic/server.go b/libnetwork/diagnostic/server.go index bab792b575..08c3e875af 100644 --- a/libnetwork/diagnostic/server.go +++ b/libnetwork/diagnostic/server.go @@ -9,7 +9,7 @@ import ( "sync/atomic" "github.com/docker/docker/libnetwork/internal/caller" - stackdump "github.com/docker/docker/pkg/signal" + "github.com/docker/docker/pkg/stack" "github.com/sirupsen/logrus" ) @@ -169,7 +169,7 @@ func stackTrace(ctx interface{}, w http.ResponseWriter, r *http.Request) { log := logrus.WithFields(logrus.Fields{"component": "diagnostic", "remoteIP": r.RemoteAddr, "method": caller.Name(0), "url": r.URL.String()}) log.Info("stack trace") - path, err := stackdump.DumpStacks("/tmp/") + path, err := stack.DumpToFile("/tmp/") if err != nil { log.WithError(err).Error("failed to write goroutines dump") HTTPReply(w, FailCommand(err), json) // nolint:errcheck diff --git a/pkg/signal/signal_deprecated.go b/pkg/signal/signal_deprecated.go new file mode 100644 index 0000000000..ded247ce35 --- /dev/null +++ b/pkg/signal/signal_deprecated.go @@ -0,0 +1,8 @@ +package signal + +import "github.com/docker/docker/pkg/stack" + +// DumpStacks appends the runtime stack into file in dir and returns full path +// to that file. +// Deprecated: use github.com/docker/docker/pkg/stack.Dump instead. +var DumpStacks = stack.DumpToFile diff --git a/pkg/signal/trap.go b/pkg/signal/trap.go index a277b95629..a838b3e7b9 100644 --- a/pkg/signal/trap.go +++ b/pkg/signal/trap.go @@ -4,14 +4,10 @@ import ( "fmt" "os" gosignal "os/signal" - "path/filepath" - "runtime" - "strings" "sync/atomic" "syscall" - "time" - "github.com/pkg/errors" + "github.com/docker/docker/pkg/stack" ) // Trap sets up a simplified signal "trap", appropriate for common @@ -58,7 +54,7 @@ func Trap(cleanup func(), logger interface { logger.Info("Forcing docker daemon shutdown without cleanup; 3 interrupts received") } case syscall.SIGQUIT: - DumpStacks("") + stack.Dump() logger.Info("Forcing docker daemon shutdown without cleanup on SIGQUIT") } // for the SIGINT/TERM, and SIGQUIT non-clean shutdown case, exit with 128 + signal # @@ -67,38 +63,3 @@ func Trap(cleanup func(), logger interface { } }() } - -const stacksLogNameTemplate = "goroutine-stacks-%s.log" - -// DumpStacks appends the runtime stack into file in dir and returns full path -// to that file. -func DumpStacks(dir string) (string, error) { - var ( - buf []byte - stackSize int - ) - bufferLen := 16384 - for stackSize == len(buf) { - buf = make([]byte, bufferLen) - stackSize = runtime.Stack(buf, true) - bufferLen *= 2 - } - buf = buf[:stackSize] - var f *os.File - if dir != "" { - path := filepath.Join(dir, fmt.Sprintf(stacksLogNameTemplate, strings.Replace(time.Now().Format(time.RFC3339), ":", "", -1))) - var err error - f, err = os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0666) - if err != nil { - return "", errors.Wrap(err, "failed to open file to write the goroutine stacks") - } - defer f.Close() - defer f.Sync() - } else { - f = os.Stderr - } - if _, err := f.Write(buf); err != nil { - return "", errors.Wrap(err, "failed to write goroutine stacks") - } - return f.Name(), nil -} diff --git a/pkg/signal/trap_linux_test.go b/pkg/signal/trap_linux_test.go index 781ca5a354..3dce0cd238 100644 --- a/pkg/signal/trap_linux_test.go +++ b/pkg/signal/trap_linux_test.go @@ -64,20 +64,3 @@ func TestTrap(t *testing.T) { } } - -func TestDumpStacks(t *testing.T) { - directory, err := ioutil.TempDir("", "test-dump-tasks") - assert.Check(t, err) - defer os.RemoveAll(directory) - dumpPath, err := DumpStacks(directory) - assert.Check(t, err) - readFile, _ := ioutil.ReadFile(dumpPath) - fileData := string(readFile) - assert.Check(t, is.Contains(fileData, "goroutine")) -} - -func TestDumpStacksWithEmptyInput(t *testing.T) { - path, err := DumpStacks("") - assert.Check(t, err) - assert.Check(t, is.Equal(os.Stderr.Name(), path)) -} diff --git a/pkg/stack/stackdump.go b/pkg/stack/stackdump.go new file mode 100644 index 0000000000..f65c186c14 --- /dev/null +++ b/pkg/stack/stackdump.go @@ -0,0 +1,57 @@ +package stack // import "github.com/docker/docker/pkg/stack" + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "strings" + "time" + + "github.com/pkg/errors" +) + +const stacksLogNameTemplate = "goroutine-stacks-%s.log" + +// Dump outputs the runtime stack to os.StdErr. +func Dump() { + _ = dump(os.Stderr) +} + +// DumpToFile appends the runtime stack into a file named "goroutine-stacks-.log" +// in dir and returns the full path to that file. If no directory name is +// provided, it outputs to os.Stderr. +func DumpToFile(dir string) (string, error) { + var f *os.File + if dir != "" { + path := filepath.Join(dir, fmt.Sprintf(stacksLogNameTemplate, strings.Replace(time.Now().Format(time.RFC3339), ":", "", -1))) + var err error + f, err = os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0666) + if err != nil { + return "", errors.Wrap(err, "failed to open file to write the goroutine stacks") + } + defer f.Close() + defer f.Sync() + } else { + f = os.Stderr + } + return f.Name(), dump(f) +} + +func dump(f *os.File) error { + var ( + buf []byte + stackSize int + ) + bufferLen := 16384 + for stackSize == len(buf) { + buf = make([]byte, bufferLen) + stackSize = runtime.Stack(buf, true) + bufferLen *= 2 + } + buf = buf[:stackSize] + if _, err := f.Write(buf); err != nil { + return errors.Wrap(err, "failed to write goroutine stacks") + } + return nil +} diff --git a/pkg/stack/stackdump_test.go b/pkg/stack/stackdump_test.go new file mode 100644 index 0000000000..ae58ba0a89 --- /dev/null +++ b/pkg/stack/stackdump_test.go @@ -0,0 +1,31 @@ +package stack // import "github.com/docker/docker/pkg/stack" + +import ( + "io/ioutil" + "os" + "testing" + + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" +) + +func TestDump(t *testing.T) { + Dump() +} + +func TestDumpToFile(t *testing.T) { + directory, err := ioutil.TempDir("", "test-dump-tasks") + assert.Check(t, err) + defer os.RemoveAll(directory) + dumpPath, err := DumpToFile(directory) + assert.Check(t, err) + readFile, _ := ioutil.ReadFile(dumpPath) + fileData := string(readFile) + assert.Check(t, is.Contains(fileData, "goroutine")) +} + +func TestDumpToFileWithEmptyInput(t *testing.T) { + path, err := DumpToFile("") + assert.Check(t, err) + assert.Check(t, is.Equal(os.Stderr.Name(), path)) +}