mirror of
https://github.com/restic/restic.git
synced 2024-12-26 01:37:12 +00:00
rclone: Fix stderr handling if command exits unexpectedly
According to the documentation of exec.Cmd Wait() must not be called before completing all reads from the pipe returned by StdErrPipe(). Thus return a context that is canceled once rclone has exited and use that as a precondition to calling Wait(). This should ensure that all errors printed to stderr have been copied first.
This commit is contained in:
parent
b8acad4da0
commit
5600f11696
1 changed files with 20 additions and 29 deletions
|
@ -38,29 +38,32 @@ type Backend struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
// run starts command with args and initializes the StdioConn.
|
// run starts command with args and initializes the StdioConn.
|
||||||
func run(command string, args ...string) (*StdioConn, *sync.WaitGroup, func() error, error) {
|
func run(command string, args ...string) (*StdioConn, *sync.WaitGroup, chan struct{}, func() error, error) {
|
||||||
cmd := exec.Command(command, args...)
|
cmd := exec.Command(command, args...)
|
||||||
|
|
||||||
p, err := cmd.StderrPipe()
|
p, err := cmd.StderrPipe()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, nil, nil, err
|
return nil, nil, nil, nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
var wg sync.WaitGroup
|
var wg sync.WaitGroup
|
||||||
|
waitCh := make(chan struct{})
|
||||||
|
|
||||||
// start goroutine to add a prefix to all messages printed by to stderr by rclone
|
// start goroutine to add a prefix to all messages printed by to stderr by rclone
|
||||||
wg.Add(1)
|
wg.Add(1)
|
||||||
go func() {
|
go func() {
|
||||||
defer wg.Done()
|
defer wg.Done()
|
||||||
|
defer close(waitCh)
|
||||||
sc := bufio.NewScanner(p)
|
sc := bufio.NewScanner(p)
|
||||||
for sc.Scan() {
|
for sc.Scan() {
|
||||||
fmt.Fprintf(os.Stderr, "rclone: %v\n", sc.Text())
|
fmt.Fprintf(os.Stderr, "rclone: %v\n", sc.Text())
|
||||||
}
|
}
|
||||||
|
debug.Log("command has exited, closing waitCh")
|
||||||
}()
|
}()
|
||||||
|
|
||||||
r, stdin, err := os.Pipe()
|
r, stdin, err := os.Pipe()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, nil, nil, err
|
return nil, nil, nil, nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
stdout, w, err := os.Pipe()
|
stdout, w, err := os.Pipe()
|
||||||
|
@ -68,7 +71,7 @@ func run(command string, args ...string) (*StdioConn, *sync.WaitGroup, func() er
|
||||||
// close first pipe and ignore subsequent errors
|
// close first pipe and ignore subsequent errors
|
||||||
_ = r.Close()
|
_ = r.Close()
|
||||||
_ = stdin.Close()
|
_ = stdin.Close()
|
||||||
return nil, nil, nil, err
|
return nil, nil, nil, nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
cmd.Stdin = r
|
cmd.Stdin = r
|
||||||
|
@ -87,9 +90,9 @@ func run(command string, args ...string) (*StdioConn, *sync.WaitGroup, func() er
|
||||||
}
|
}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if backend.IsErrDot(err) {
|
if backend.IsErrDot(err) {
|
||||||
return nil, nil, nil, errors.Errorf("cannot implicitly run relative executable %v found in current directory, use -o rclone.program=./<program> to override", cmd.Path)
|
return nil, nil, nil, nil, errors.Errorf("cannot implicitly run relative executable %v found in current directory, use -o rclone.program=./<program> to override", cmd.Path)
|
||||||
}
|
}
|
||||||
return nil, nil, nil, err
|
return nil, nil, nil, nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
c := &StdioConn{
|
c := &StdioConn{
|
||||||
|
@ -98,7 +101,7 @@ func run(command string, args ...string) (*StdioConn, *sync.WaitGroup, func() er
|
||||||
cmd: cmd,
|
cmd: cmd,
|
||||||
}
|
}
|
||||||
|
|
||||||
return c, &wg, bg, nil
|
return c, &wg, waitCh, bg, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// wrappedConn adds bandwidth limiting capabilities to the StdioConn by
|
// wrappedConn adds bandwidth limiting capabilities to the StdioConn by
|
||||||
|
@ -162,7 +165,7 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) {
|
||||||
arg0, args := args[0], args[1:]
|
arg0, args := args[0], args[1:]
|
||||||
|
|
||||||
debug.Log("running command: %v %v", arg0, args)
|
debug.Log("running command: %v %v", arg0, args)
|
||||||
stdioConn, wg, bg, err := run(arg0, args...)
|
stdioConn, wg, waitCh, bg, err := run(arg0, args...)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
@ -187,7 +190,6 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) {
|
||||||
}
|
}
|
||||||
|
|
||||||
cmd := stdioConn.cmd
|
cmd := stdioConn.cmd
|
||||||
waitCh := make(chan struct{})
|
|
||||||
be := &Backend{
|
be := &Backend{
|
||||||
tr: tr,
|
tr: tr,
|
||||||
cmd: cmd,
|
cmd: cmd,
|
||||||
|
@ -196,32 +198,21 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) {
|
||||||
wg: wg,
|
wg: wg,
|
||||||
}
|
}
|
||||||
|
|
||||||
wg.Add(1)
|
|
||||||
go func() {
|
|
||||||
defer wg.Done()
|
|
||||||
debug.Log("waiting for error result")
|
|
||||||
err := cmd.Wait()
|
|
||||||
debug.Log("Wait returned %v", err)
|
|
||||||
be.waitResult = err
|
|
||||||
// close our side of the pipes to rclone, ignore errors
|
|
||||||
_ = stdioConn.CloseAll()
|
|
||||||
close(waitCh)
|
|
||||||
}()
|
|
||||||
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
defer cancel()
|
defer cancel()
|
||||||
|
|
||||||
wg.Add(1)
|
wg.Add(1)
|
||||||
go func() {
|
go func() {
|
||||||
defer wg.Done()
|
defer wg.Done()
|
||||||
debug.Log("monitoring command to cancel first HTTP request context")
|
<-waitCh
|
||||||
select {
|
cancel()
|
||||||
case <-ctx.Done():
|
|
||||||
debug.Log("context has been cancelled, returning")
|
// according to the documentation of StdErrPipe, Wait() must only be called after the former has completed
|
||||||
case <-be.waitCh:
|
err := cmd.Wait()
|
||||||
debug.Log("command has exited, cancelling context")
|
debug.Log("Wait returned %v", err)
|
||||||
cancel()
|
be.waitResult = err
|
||||||
}
|
// close our side of the pipes to rclone, ignore errors
|
||||||
|
_ = stdioConn.CloseAll()
|
||||||
}()
|
}()
|
||||||
|
|
||||||
// send an HTTP request to the base URL, see if the server is there
|
// send an HTTP request to the base URL, see if the server is there
|
||||||
|
|
Loading…
Reference in a new issue