From 0c3467ffb719eed9ed1d6a42be9491af44a1d14d Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 20 Jul 2021 22:52:19 +0100 Subject: [PATCH] Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end (#16479) (#16480) * Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end (#16479) Fix #16427 (again!) * handle sharing violation error code Signed-off-by: Andrew Thornton --- modules/git/batch_reader.go | 15 +++++++++++++-- modules/util/remove.go | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index bdf82bde891..164e6438128 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -7,6 +7,7 @@ package git import ( "bufio" "bytes" + "context" "io" "math" "strconv" @@ -28,16 +29,20 @@ type WriteCloserError interface { func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()) { batchStdinReader, batchStdinWriter := io.Pipe() batchStdoutReader, batchStdoutWriter := io.Pipe() + ctx, ctxCancel := context.WithCancel(DefaultContext) + closed := make(chan struct{}) cancel := func() { _ = batchStdinReader.Close() _ = batchStdinWriter.Close() _ = batchStdoutReader.Close() _ = batchStdoutWriter.Close() + ctxCancel() + <-closed } go func() { stderr := strings.Builder{} - err := NewCommand("cat-file", "--batch-check").RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader) + err := NewCommandContext(ctx, "cat-file", "--batch-check").RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader) if err != nil { _ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) _ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String())) @@ -45,6 +50,7 @@ func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func() _ = batchStdoutWriter.Close() _ = batchStdinReader.Close() } + close(closed) }() // For simplicities sake we'll use a buffered reader to read from the cat-file --batch-check @@ -59,16 +65,20 @@ func CatFileBatch(repoPath string) (WriteCloserError, *bufio.Reader, func()) { // so let's create a batch stdin and stdout batchStdinReader, batchStdinWriter := io.Pipe() batchStdoutReader, batchStdoutWriter := nio.Pipe(buffer.New(32 * 1024)) + ctx, ctxCancel := context.WithCancel(DefaultContext) + closed := make(chan struct{}) cancel := func() { _ = batchStdinReader.Close() _ = batchStdinWriter.Close() _ = batchStdoutReader.Close() _ = batchStdoutWriter.Close() + ctxCancel() + <-closed } go func() { stderr := strings.Builder{} - err := NewCommand("cat-file", "--batch").RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader) + err := NewCommandContext(ctx, "cat-file", "--batch").RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader) if err != nil { _ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) _ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String())) @@ -76,6 +86,7 @@ func CatFileBatch(repoPath string) (WriteCloserError, *bufio.Reader, func()) { _ = batchStdoutWriter.Close() _ = batchStdinReader.Close() } + close(closed) }() // For simplicities sake we'll us a buffered reader to read from the cat-file --batch diff --git a/modules/util/remove.go b/modules/util/remove.go index 754f3b7c117..d05ee9fe4af 100644 --- a/modules/util/remove.go +++ b/modules/util/remove.go @@ -6,10 +6,13 @@ package util import ( "os" + "runtime" "syscall" "time" ) +const windowsSharingViolationError syscall.Errno = 32 + // Remove removes the named file or (empty) directory with at most 5 attempts. func Remove(name string) error { var err error @@ -25,6 +28,12 @@ func Remove(name string) error { continue } + if unwrapped == windowsSharingViolationError && runtime.GOOS == "windows" { + // try again + <-time.After(100 * time.Millisecond) + continue + } + if unwrapped == syscall.ENOENT { // it's already gone return nil @@ -48,6 +57,12 @@ func RemoveAll(name string) error { continue } + if unwrapped == windowsSharingViolationError && runtime.GOOS == "windows" { + // try again + <-time.After(100 * time.Millisecond) + continue + } + if unwrapped == syscall.ENOENT { // it's already gone return nil @@ -71,6 +86,12 @@ func Rename(oldpath, newpath string) error { continue } + if unwrapped == windowsSharingViolationError && runtime.GOOS == "windows" { + // try again + <-time.After(100 * time.Millisecond) + continue + } + if i == 0 && os.IsNotExist(err) { return err }