From 4e5caab11415cf332a42beae4d5c7e84fb46c4de Mon Sep 17 00:00:00 2001
From: Enrico204 <enrico204@gmail.com>
Date: Sun, 27 Aug 2023 09:57:02 +0200
Subject: [PATCH] stdin-from-command: implemented suggestions in #4254

The code has been refactored so that the archiver is back to the original code, and the stderr is handled using a go routine to avoid deadlock.
---
 cmd/restic/cmd_backup.go         | 50 ++++++++++++++++++++------------
 internal/archiver/archiver.go    | 13 ---------
 internal/fs/fs_reader_command.go | 23 +++++++++++++++
 3 files changed, 55 insertions(+), 31 deletions(-)
 create mode 100644 internal/fs/fs_reader_command.go

diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go
index 798d5609c..af736d13b 100644
--- a/cmd/restic/cmd_backup.go
+++ b/cmd/restic/cmd_backup.go
@@ -301,7 +301,7 @@ func (opts BackupOptions) Check(gopts GlobalOptions, args []string) error {
 			return errors.Fatal("--stdin and --files-from-raw cannot be used together")
 		}
 
-		if len(args) > 0 && opts.StdinCommand == false {
+		if len(args) > 0 && !opts.StdinCommand {
 			return errors.Fatal("--stdin was specified and files/dirs were listed as arguments")
 		}
 	}
@@ -596,30 +596,17 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 		targetFS = localVss
 	}
 
-	var command *exec.Cmd
-	var stderr io.ReadCloser
 	if opts.Stdin || opts.StdinCommand {
 		if !gopts.JSON {
 			progressPrinter.V("read data from stdin")
 		}
 		filename := path.Join("/", opts.StdinFilename)
-		var closer io.ReadCloser
+		var closer io.ReadCloser = os.Stdin
 		if opts.StdinCommand {
-			command = exec.CommandContext(ctx, args[0], args[1:]...)
-			stdout, err := command.StdoutPipe()
+			closer, err = prepareStdinCommand(ctx, args)
 			if err != nil {
 				return err
 			}
-			stderr, err = command.StderrPipe()
-			if err != nil {
-				return err
-			}
-			if err := command.Start(); err != nil {
-				return err
-			}
-			closer = stdout
-		} else {
-			closer = os.Stdin
 		}
 		targetFS = &fs.Reader{
 			ModTime:    timeStamp,
@@ -676,8 +663,6 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 		Hostname:       opts.Host,
 		ParentSnapshot: parentSnapshot,
 		ProgramVersion: "restic " + version,
-		Command:        command,
-		CommandStderr:  stderr,
 	}
 
 	if !gopts.JSON {
@@ -708,3 +693,32 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 	// Return error if any
 	return werr
 }
+
+func prepareStdinCommand(ctx context.Context, args []string) (io.ReadCloser, error) {
+	// Prepare command and stdout. These variables will be assigned to the
+	// io.ReadCloser that is used by the archiver to read data, so that the
+	// Close() function waits for the program to finish. See
+	// fs.ReadCloserCommand.
+	command := exec.CommandContext(ctx, args[0], args[1:]...)
+	stdout, err := command.StdoutPipe()
+	if err != nil {
+		return nil, errors.Wrap(err, "command.StdoutPipe")
+	}
+
+	// Use a Go routine to handle the stderr to avoid deadlocks
+	stderr, err := command.StderrPipe()
+	if err != nil {
+		return nil, errors.Wrap(err, "command.StderrPipe")
+	}
+	go func() {
+		sc := bufio.NewScanner(stderr)
+		for sc.Scan() {
+			_, _ = fmt.Fprintf(os.Stderr, "subprocess %v: %v\n", command.Args[0], sc.Text())
+		}
+	}()
+
+	if err := command.Start(); err != nil {
+		return nil, errors.Wrap(err, "command.Start")
+	}
+	return &fs.ReadCloserCommand{Cmd: command, Stdout: stdout}, nil
+}
diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go
index 91f991e7d..98819d797 100644
--- a/internal/archiver/archiver.go
+++ b/internal/archiver/archiver.go
@@ -2,9 +2,7 @@ package archiver
 
 import (
 	"context"
-	"io"
 	"os"
-	"os/exec"
 	"path"
 	"runtime"
 	"sort"
@@ -683,8 +681,6 @@ type SnapshotOptions struct {
 	Time           time.Time
 	ParentSnapshot *restic.Snapshot
 	ProgramVersion string
-	Command        *exec.Cmd
-	CommandStderr  io.ReadCloser
 }
 
 // loadParentTree loads a tree referenced by snapshot id. If id is null, nil is returned.
@@ -796,15 +792,6 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps
 		return nil, restic.ID{}, err
 	}
 
-	if opts.Command != nil {
-		errBytes, _ := io.ReadAll(opts.CommandStderr)
-		cmdErr := opts.Command.Wait()
-		if cmdErr != nil {
-			debug.Log("error while executing command: %v", cmdErr)
-			return nil, restic.ID{}, errors.New(string(errBytes))
-		}
-	}
-
 	sn, err := restic.NewSnapshot(targets, opts.Tags, opts.Hostname, opts.Time)
 	if err != nil {
 		return nil, restic.ID{}, err
diff --git a/internal/fs/fs_reader_command.go b/internal/fs/fs_reader_command.go
new file mode 100644
index 000000000..f35454c7f
--- /dev/null
+++ b/internal/fs/fs_reader_command.go
@@ -0,0 +1,23 @@
+package fs
+
+import (
+	"io"
+	"os/exec"
+)
+
+// ReadCloserCommand wraps an exec.Cmd and its standard output to provide an
+// io.ReadCloser that waits for the command to terminate on Close(), reporting
+// any error in the command.Wait() function back to the Close() caller.
+type ReadCloserCommand struct {
+	Cmd    *exec.Cmd
+	Stdout io.ReadCloser
+}
+
+func (fp *ReadCloserCommand) Read(p []byte) (n int, err error) {
+	return fp.Stdout.Read(p)
+}
+
+func (fp *ReadCloserCommand) Close() error {
+	// No need to close fp.Stdout as Wait() closes all pipes.
+	return fp.Cmd.Wait()
+}