From 1ba5a4c5492b58a737a0a05c30eff7adfa85955f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 18 Jul 2021 21:37:58 +0100 Subject: [PATCH 1/3] Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end Fix #16427 (again!) Signed-off-by: Andrew Thornton --- modules/git/batch_reader.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index bdf82bde8913..164e64381289 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 From 5d2968b9757f3be6b00937b0dba90ac5ebf55d95 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 19 Jul 2021 16:50:04 +0100 Subject: [PATCH 2/3] handle sharing violation error code Signed-off-by: Andrew Thornton --- modules/util/remove.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/modules/util/remove.go b/modules/util/remove.go index 754f3b7c1174..71e2529c12eb 100644 --- a/modules/util/remove.go +++ b/modules/util/remove.go @@ -6,10 +6,13 @@ package util import ( "os" + "runtime" "syscall" "time" ) +const WIN_ERROR_SHARING_VIOLATION 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 == WIN_ERROR_SHARING_VIOLATION && 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 == WIN_ERROR_SHARING_VIOLATION && 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 == WIN_ERROR_SHARING_VIOLATION && runtime.GOOS == "windows" { + // try again + <-time.After(100 * time.Millisecond) + continue + } + if i == 0 && os.IsNotExist(err) { return err } From 5649e8e255ca39fe24bbacb8b912be65438115d9 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 19 Jul 2021 18:53:48 +0100 Subject: [PATCH 3/3] placate lint Signed-off-by: Andrew Thornton --- modules/util/remove.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/util/remove.go b/modules/util/remove.go index 71e2529c12eb..d05ee9fe4afa 100644 --- a/modules/util/remove.go +++ b/modules/util/remove.go @@ -11,7 +11,7 @@ import ( "time" ) -const WIN_ERROR_SHARING_VIOLATION syscall.Errno = 32 +const windowsSharingViolationError syscall.Errno = 32 // Remove removes the named file or (empty) directory with at most 5 attempts. func Remove(name string) error { @@ -28,7 +28,7 @@ func Remove(name string) error { continue } - if unwrapped == WIN_ERROR_SHARING_VIOLATION && runtime.GOOS == "windows" { + if unwrapped == windowsSharingViolationError && runtime.GOOS == "windows" { // try again <-time.After(100 * time.Millisecond) continue @@ -57,7 +57,7 @@ func RemoveAll(name string) error { continue } - if unwrapped == WIN_ERROR_SHARING_VIOLATION && runtime.GOOS == "windows" { + if unwrapped == windowsSharingViolationError && runtime.GOOS == "windows" { // try again <-time.After(100 * time.Millisecond) continue @@ -86,7 +86,7 @@ func Rename(oldpath, newpath string) error { continue } - if unwrapped == WIN_ERROR_SHARING_VIOLATION && runtime.GOOS == "windows" { + if unwrapped == windowsSharingViolationError && runtime.GOOS == "windows" { // try again <-time.After(100 * time.Millisecond) continue