From 3cd927d180d8e514261d4f69f180f3c4d37c9bf0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 24 Jul 2020 23:27:47 +0200 Subject: [PATCH] rclone: Give rclone time to finish before closing stdin pipe Calling `Close()` on the rclone backend sometimes failed during test execution with 'signal: Broken pipe'. The stdio connection closed both the stdin and stdout file descriptors at the same moment, therefore giving rclone no chance to properly send any final http2 data frames. Now the stdin connection to rclone is closed first and will only be forcefully closed after a timeout. In case rclone exits before the timeout then the stdio connection will be closed normally. --- internal/backend/rclone/backend.go | 4 +-- internal/backend/rclone/stdio_conn.go | 37 +++++++++++++++------------ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/internal/backend/rclone/backend.go b/internal/backend/rclone/backend.go index 5d7e62871..c577b0b26 100644 --- a/internal/backend/rclone/backend.go +++ b/internal/backend/rclone/backend.go @@ -197,7 +197,7 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) { debug.Log("Wait returned %v", err) be.waitResult = err // close our side of the pipes to rclone - stdioConn.Close() + stdioConn.CloseAll() close(waitCh) }() @@ -314,7 +314,7 @@ func (be *Backend) Close() error { debug.Log("rclone exited") case <-time.After(waitForExit): debug.Log("timeout, closing file descriptors") - err := be.conn.Close() + err := be.conn.CloseAll() if err != nil { return err } diff --git a/internal/backend/rclone/stdio_conn.go b/internal/backend/rclone/stdio_conn.go index a44e6aa5c..ac8dcc65a 100644 --- a/internal/backend/rclone/stdio_conn.go +++ b/internal/backend/rclone/stdio_conn.go @@ -12,10 +12,11 @@ import ( // StdioConn implements a net.Conn via stdin/stdout. type StdioConn struct { - stdin *os.File - stdout *os.File - cmd *exec.Cmd - close sync.Once + stdin *os.File + stdout *os.File + cmd *exec.Cmd + closeIn sync.Once + closeOut sync.Once } func (s *StdioConn) Read(p []byte) (int, error) { @@ -28,21 +29,25 @@ func (s *StdioConn) Write(p []byte) (int, error) { return n, err } -// Close closes both streams. +// Close closes the stream to the child process. func (s *StdioConn) Close() (err error) { - s.close.Do(func() { - debug.Log("close stdio connection") - var errs []error + s.closeOut.Do(func() { + debug.Log("close stdio send connection") + err = s.stdout.Close() + }) - for _, f := range []func() error{s.stdin.Close, s.stdout.Close} { - err := f() - if err != nil { - errs = append(errs, err) - } - } + return err +} - if len(errs) > 0 { - err = errs[0] +// CloseAll closes both streams. +func (s *StdioConn) CloseAll() (err error) { + err = s.Close() + + s.closeIn.Do(func() { + debug.Log("close stdio receive connection") + err2 := s.stdin.Close() + if err == nil { + err = err2 } })