From e4c469c149becc5f4abb9b77c03dd1f46be7a301 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Mon, 28 Aug 2017 21:54:01 -0700 Subject: [PATCH] debug: properly handle interrupted profiles By default (i.e., without profile.NoShutdownHook), profile.Start listens for SIGINT and will stop the profile and call os.Exit(0). restic already listens for SIGINT and runs its own cleanup handlers before calling os.Exit(0). As is, these handlers are racing when an interrupt occurs, and in my experience, restic tends to win the race, resulting in an unusable profile. Eliminate the race and properly stop profiles on interrupt by disabling package profile's signal handler and instead stop the profile in a restic cleanup handler. --- cmd/restic/global_debug.go | 25 +++++++++++++------------ cmd/restic/global_release.go | 3 --- cmd/restic/main.go | 3 --- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/cmd/restic/global_debug.go b/cmd/restic/global_debug.go index 1ba34f8b5..7cad172f6 100644 --- a/cmd/restic/global_debug.go +++ b/cmd/restic/global_debug.go @@ -19,10 +19,6 @@ var ( memProfilePath string cpuProfilePath string insecure bool - - prof interface { - Stop() - } ) func init() { @@ -54,10 +50,21 @@ func runDebug() error { return errors.Fatal("only one profile (memory or CPU) may be activated at the same time") } + var prof interface { + Stop() + } + if memProfilePath != "" { - prof = profile.Start(profile.Quiet, profile.MemProfile, profile.ProfilePath(memProfilePath)) + prof = profile.Start(profile.Quiet, profile.NoShutdownHook, profile.MemProfile, profile.ProfilePath(memProfilePath)) } else if cpuProfilePath != "" { - prof = profile.Start(profile.Quiet, profile.CPUProfile, profile.ProfilePath(cpuProfilePath)) + prof = profile.Start(profile.Quiet, profile.NoShutdownHook, profile.CPUProfile, profile.ProfilePath(cpuProfilePath)) + } + + if prof != nil { + AddCleanupHandler(func() error { + prof.Stop() + return nil + }) } if insecure { @@ -66,9 +73,3 @@ func runDebug() error { return nil } - -func shutdownDebug() { - if prof != nil { - prof.Stop() - } -} diff --git a/cmd/restic/global_release.go b/cmd/restic/global_release.go index 0a3bc8f1b..04c7cba31 100644 --- a/cmd/restic/global_release.go +++ b/cmd/restic/global_release.go @@ -4,6 +4,3 @@ package main // runDebug is a noop without the debug tag. func runDebug() error { return nil } - -// shutdownDebug is a noop without the debug tag. -func shutdownDebug() {} diff --git a/cmd/restic/main.go b/cmd/restic/main.go index d44099193..602928447 100644 --- a/cmd/restic/main.go +++ b/cmd/restic/main.go @@ -52,9 +52,6 @@ directories in an encrypted repository stored on different backends. return nil }, - PersistentPostRun: func(*cobra.Command, []string) { - shutdownDebug() - }, } var logBuffer = bytes.NewBuffer(nil)