From 1a1c572bacbfa422f4aa18b4abc9a0fb8c98559e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 12 Apr 2020 18:46:22 +0200 Subject: [PATCH 1/2] Fix shutdown hang when restic is started as background job restic uses a cleanup hook to ensure that it restores the terminal configuration to a sane state, when restic is interrupted while reading a password from the terminal. However, this causes a problem, when restic runs in a background job, as reconfiguring a terminal will cause a SIGTTOU to be sent to restic pausing it. Therefore, restic seems to hang on shutdown when it was running in the background. This commit changes the behavior to only restore the terminal configuration if restic was interrupted while reading a password from the terminal. As reading a password from the terminal requires that restic is in the foreground, this should avoid restic getting stopped. Fixes #2298 Issue introduced in #402 --- cmd/restic/global.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 2c4d1fccd..4eb73d2bf 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -85,6 +85,8 @@ var globalOptions = GlobalOptions{ stderr: os.Stderr, } +var isReadingPassword bool + func init() { var cancel context.CancelFunc globalOptions.ctx, cancel = context.WithCancel(context.Background()) @@ -146,7 +148,10 @@ func stdoutTerminalWidth() int { } // restoreTerminal installs a cleanup handler that restores the previous -// terminal state on exit. +// terminal state on exit. This handler is only intended to restore the +// terminal configuration if restic exits after receiving a signal. A regular +// program execution must revert changes to the terminal configuration itself. +// The terminal configuration is only restored while reading a password. func restoreTerminal() { if !stdoutIsTerminal() { return @@ -160,9 +165,17 @@ func restoreTerminal() { } AddCleanupHandler(func() error { + // Restoring the terminal configuration while restic runs in the + // background, causes restic to get stopped on unix systems with + // a SIGTTOU signal. Thus only restore the terminal settings if + // they might have been modified, which is the case while reading + // a password. + if !isReadingPassword { + return nil + } err := checkErrno(terminal.Restore(fd, state)) if err != nil { - fmt.Fprintf(os.Stderr, "unable to get restore terminal state: %#+v\n", err) + fmt.Fprintf(os.Stderr, "unable to restore terminal state: %v\n", err) } return err }) @@ -302,7 +315,9 @@ func readPassword(in io.Reader) (password string, err error) { // password. func readPasswordTerminal(in *os.File, out io.Writer, prompt string) (password string, err error) { fmt.Fprint(out, prompt) + isReadingPassword = true buf, err := terminal.ReadPassword(int(in.Fd())) + isReadingPassword = false fmt.Fprintln(out) if err != nil { return "", errors.Wrap(err, "ReadPassword") From 4a400f94bb3362002776660ad7e07c886d850c58 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 12 Apr 2020 19:24:23 +0200 Subject: [PATCH 2/2] Add changelog --- changelog/unreleased/issue-2298 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/issue-2298 diff --git a/changelog/unreleased/issue-2298 b/changelog/unreleased/issue-2298 new file mode 100644 index 000000000..5da1f108f --- /dev/null +++ b/changelog/unreleased/issue-2298 @@ -0,0 +1,8 @@ +Bugfix: Do not hang when run as a background job + +Restic did hang on exit while restoring the terminal configuration when it was +started as a background job, for example using `restic ... &`. This has been +fixed by only restoring the terminal configuration when restic is interrupted +while reading a password from the terminal. + +https://github.com/restic/restic/issues/2298