From 1ed93bd54d7988a33bb91c1afc465728eeee071e Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sun, 7 Sep 2025 12:57:01 +0200 Subject: [PATCH] backend,termstatus: Unify foreground/background detection PR #5358 reintroduced a version of the TIOCGPGRP ioctl call that works on all Unix platforms except Linux, due to a bug/inconsistency in x/sys/unix. This commit fixes that by introducing termstatus.Tcgetpgrp. It also introduces termstatus.Getpgrp and termstatus.Tcsetpgrp to deal with the different signature of unix.Getpgrp in Solaris vs. all other Unix platforms and an int-overflowing constant on AIX, so that some AIX/Solaris-specific code can be removed elsewhere and foreground/background detection is done the same everywhere except on Windows. --- internal/backend/util/foreground_sysv.go | 28 ------------------- internal/backend/util/foreground_unix.go | 24 +++++----------- internal/ui/termstatus/background_linux.go | 27 ------------------ internal/ui/termstatus/background_unix.go | 24 ++++++++++++++++ ..._linux_test.go => background_unix_test.go} | 4 ++- .../{background.go => background_windows.go} | 3 -- internal/ui/termstatus/getpgrp_solaris.go | 8 ++++++ internal/ui/termstatus/getpgrp_unix.go | 7 +++++ internal/ui/termstatus/tcgetpgrp_linux.go | 12 ++++++++ internal/ui/termstatus/tcgetpgrp_unix.go | 9 ++++++ internal/ui/termstatus/tcsetpgrp_aix.go | 10 +++++++ internal/ui/termstatus/tcsetpgrp_unix.go | 9 ++++++ 12 files changed, 89 insertions(+), 76 deletions(-) delete mode 100644 internal/backend/util/foreground_sysv.go delete mode 100644 internal/ui/termstatus/background_linux.go create mode 100644 internal/ui/termstatus/background_unix.go rename internal/ui/termstatus/{background_linux_test.go => background_unix_test.go} (81%) rename internal/ui/termstatus/{background.go => background_windows.go} (85%) create mode 100644 internal/ui/termstatus/getpgrp_solaris.go create mode 100644 internal/ui/termstatus/getpgrp_unix.go create mode 100644 internal/ui/termstatus/tcgetpgrp_linux.go create mode 100644 internal/ui/termstatus/tcgetpgrp_unix.go create mode 100644 internal/ui/termstatus/tcsetpgrp_aix.go create mode 100644 internal/ui/termstatus/tcsetpgrp_unix.go diff --git a/internal/backend/util/foreground_sysv.go b/internal/backend/util/foreground_sysv.go deleted file mode 100644 index ec06aa677..000000000 --- a/internal/backend/util/foreground_sysv.go +++ /dev/null @@ -1,28 +0,0 @@ -//go:build aix || solaris -// +build aix solaris - -package util - -import ( - "os/exec" - "syscall" - - "github.com/restic/restic/internal/errors" -) - -func startForeground(cmd *exec.Cmd) (bg func() error, err error) { - // run the command in its own process group so that SIGINT - // is not sent to it. - cmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, - } - - // start the process - err = cmd.Start() - if err != nil { - return nil, errors.Wrap(err, "cmd.Start") - } - - bg = func() error { return nil } - return bg, nil -} diff --git a/internal/backend/util/foreground_unix.go b/internal/backend/util/foreground_unix.go index cba22ab26..3952885f8 100644 --- a/internal/backend/util/foreground_unix.go +++ b/internal/backend/util/foreground_unix.go @@ -1,5 +1,4 @@ -//go:build !aix && !solaris && !windows -// +build !aix,!solaris,!windows +//go:build unix package util @@ -10,20 +9,11 @@ import ( "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/ui/termstatus" "golang.org/x/sys/unix" ) -func tcgetpgrp(fd int) (int, error) { - return unix.IoctlGetInt(fd, unix.TIOCGPGRP) -} - -func tcsetpgrp(fd int, pid int) error { - // IoctlSetPointerInt silently casts to int32 internally, - // so this assumes pid fits in 31 bits. - return unix.IoctlSetPointerInt(fd, unix.TIOCSPGRP, pid) -} - func startForeground(cmd *exec.Cmd) (bg func() error, err error) { // run the command in its own process group // this ensures that sending ctrl-c to restic will not immediately stop the backend process. @@ -39,15 +29,15 @@ func startForeground(cmd *exec.Cmd) (bg func() error, err error) { } // only move child process to foreground if restic is in the foreground - prev, err := tcgetpgrp(int(tty.Fd())) + prev, err := termstatus.Tcgetpgrp(int(tty.Fd())) if err != nil { _ = tty.Close() return nil, err } - self := unix.Getpgrp() + self := termstatus.Getpgrp() if prev != self { - debug.Log("restic is not controlling the tty") + debug.Log("restic is not controlling the tty; err = %v", err) if err := tty.Close(); err != nil { return nil, err } @@ -66,7 +56,7 @@ func startForeground(cmd *exec.Cmd) (bg func() error, err error) { } // move the command's process group into the foreground - err = tcsetpgrp(int(tty.Fd()), cmd.Process.Pid) + err = termstatus.Tcsetpgrp(int(tty.Fd()), cmd.Process.Pid) if err != nil { _ = tty.Close() return nil, err @@ -77,7 +67,7 @@ func startForeground(cmd *exec.Cmd) (bg func() error, err error) { signal.Reset(unix.SIGTTOU) // reset the foreground process group - err = tcsetpgrp(int(tty.Fd()), prev) + err = termstatus.Tcsetpgrp(int(tty.Fd()), prev) if err != nil { _ = tty.Close() return err diff --git a/internal/ui/termstatus/background_linux.go b/internal/ui/termstatus/background_linux.go deleted file mode 100644 index db96c2c53..000000000 --- a/internal/ui/termstatus/background_linux.go +++ /dev/null @@ -1,27 +0,0 @@ -package termstatus - -import ( - "github.com/restic/restic/internal/debug" - - "golang.org/x/sys/unix" -) - -// IsProcessBackground reports whether the current process is running in the -// background. fd must be a file descriptor for the terminal. -func IsProcessBackground(fd uintptr) bool { - bg, err := isProcessBackground(fd) - if err != nil { - debug.Log("Can't check if we are in the background. Using default behaviour. Error: %s\n", err.Error()) - return false - } - return bg -} - -func isProcessBackground(fd uintptr) (bool, error) { - // We need to use IoctlGetUint32 here, because pid_t is 32-bit even on - // 64-bit Linux. IoctlGetInt doesn't work on big-endian platforms: - // https://github.com/golang/go/issues/45585 - // https://github.com/golang/go/issues/60429 - pid, err := unix.IoctlGetUint32(int(fd), unix.TIOCGPGRP) - return int(pid) != unix.Getpgrp(), err -} diff --git a/internal/ui/termstatus/background_unix.go b/internal/ui/termstatus/background_unix.go new file mode 100644 index 000000000..666e42dd1 --- /dev/null +++ b/internal/ui/termstatus/background_unix.go @@ -0,0 +1,24 @@ +//go:build unix + +package termstatus + +import "github.com/restic/restic/internal/debug" + +// IsProcessBackground reports whether the current process is running in the +// background. fd must be a file descriptor for the terminal. +func IsProcessBackground(fd uintptr) bool { + bg, err := isProcessBackground(int(fd)) + if err != nil { + debug.Log("Can't check if we are in the background. Using default behaviour. Error: %s\n", err.Error()) + return false + } + return bg +} + +func isProcessBackground(fd int) (bg bool, err error) { + pgid, err := Tcgetpgrp(fd) + if err != nil { + return false, err + } + return pgid != Getpgrp(), nil +} diff --git a/internal/ui/termstatus/background_linux_test.go b/internal/ui/termstatus/background_unix_test.go similarity index 81% rename from internal/ui/termstatus/background_linux_test.go rename to internal/ui/termstatus/background_unix_test.go index f5796b23b..301e780de 100644 --- a/internal/ui/termstatus/background_linux_test.go +++ b/internal/ui/termstatus/background_unix_test.go @@ -1,3 +1,5 @@ +//go:build unix + package termstatus import ( @@ -13,7 +15,7 @@ func TestIsProcessBackground(t *testing.T) { t.Skipf("can't open terminal: %v", err) } - _, err = isProcessBackground(tty.Fd()) + _, err = isProcessBackground(int(tty.Fd())) rtest.OK(t, err) _ = tty.Close() diff --git a/internal/ui/termstatus/background.go b/internal/ui/termstatus/background_windows.go similarity index 85% rename from internal/ui/termstatus/background.go rename to internal/ui/termstatus/background_windows.go index d4b918ae7..b210509e7 100644 --- a/internal/ui/termstatus/background.go +++ b/internal/ui/termstatus/background_windows.go @@ -1,6 +1,3 @@ -//go:build !linux -// +build !linux - package termstatus // IsProcessBackground reports whether the current process is running in the diff --git a/internal/ui/termstatus/getpgrp_solaris.go b/internal/ui/termstatus/getpgrp_solaris.go new file mode 100644 index 000000000..1df46d93c --- /dev/null +++ b/internal/ui/termstatus/getpgrp_solaris.go @@ -0,0 +1,8 @@ +package termstatus + +import "golang.org/x/sys/unix" + +func Getpgrp() int { + pid, _ := unix.Getpgrp() + return pid +} diff --git a/internal/ui/termstatus/getpgrp_unix.go b/internal/ui/termstatus/getpgrp_unix.go new file mode 100644 index 000000000..f5a7b213f --- /dev/null +++ b/internal/ui/termstatus/getpgrp_unix.go @@ -0,0 +1,7 @@ +//go:build unix && !solaris + +package termstatus + +import "golang.org/x/sys/unix" + +func Getpgrp() int { return unix.Getpgrp() } diff --git a/internal/ui/termstatus/tcgetpgrp_linux.go b/internal/ui/termstatus/tcgetpgrp_linux.go new file mode 100644 index 000000000..51a9fd2da --- /dev/null +++ b/internal/ui/termstatus/tcgetpgrp_linux.go @@ -0,0 +1,12 @@ +package termstatus + +import "golang.org/x/sys/unix" + +func Tcgetpgrp(ttyfd int) (int, error) { + // We need to use IoctlGetUint32 here, because pid_t is 32-bit even on + // 64-bit Linux. IoctlGetInt doesn't work on big-endian platforms: + // https://github.com/golang/go/issues/45585 + // https://github.com/golang/go/issues/60429 + pid, err := unix.IoctlGetUint32(ttyfd, unix.TIOCGPGRP) + return int(pid), err +} diff --git a/internal/ui/termstatus/tcgetpgrp_unix.go b/internal/ui/termstatus/tcgetpgrp_unix.go new file mode 100644 index 000000000..1359b5e46 --- /dev/null +++ b/internal/ui/termstatus/tcgetpgrp_unix.go @@ -0,0 +1,9 @@ +//go:build unix && !linux + +package termstatus + +import "golang.org/x/sys/unix" + +func Tcgetpgrp(ttyfd int) (int, error) { + return unix.IoctlGetInt(ttyfd, unix.TIOCGPGRP) +} diff --git a/internal/ui/termstatus/tcsetpgrp_aix.go b/internal/ui/termstatus/tcsetpgrp_aix.go new file mode 100644 index 000000000..8f2a5cab1 --- /dev/null +++ b/internal/ui/termstatus/tcsetpgrp_aix.go @@ -0,0 +1,10 @@ +package termstatus + +import "golang.org/x/sys/unix" + +func Tcsetpgrp(fd int, pid int) error { + // The second argument to IoctlSetPointerInt has type int on AIX, + // but the constant overflows 64-bit int, hence the two-step cast. + req := uint(unix.TIOCSPGRP) + return unix.IoctlSetPointerInt(fd, int(req), pid) +} diff --git a/internal/ui/termstatus/tcsetpgrp_unix.go b/internal/ui/termstatus/tcsetpgrp_unix.go new file mode 100644 index 000000000..7762ee448 --- /dev/null +++ b/internal/ui/termstatus/tcsetpgrp_unix.go @@ -0,0 +1,9 @@ +//go:build unix && !aix + +package termstatus + +import "golang.org/x/sys/unix" + +func Tcsetpgrp(fd int, pid int) error { + return unix.IoctlSetPointerInt(fd, unix.TIOCSPGRP, pid) +}