Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end (#16479) (#16481)

* Make cancel from CatFileBatch and CatFileBatchCheck wait for the command to end

Fix #16427 (again!)

* handle sharing violation error code

Signed-off-by: Andrew Thornton <art27@cantab.net>

2 files changed, 30 insertions, 3 deletions

diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go
index 65857f018..a3288872f 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"
@@ -15,20 +16,24 @@ import (
 
 // CatFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function
 func CatFileBatch(repoPath string) (*io.PipeWriter, *bufio.Reader, func()) {
-	// Next feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
+	// We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary.
 	// so let's create a batch stdin and stdout
 	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").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()))
@@ -36,10 +41,11 @@ func CatFileBatch(repoPath string) (*io.PipeWriter, *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
-	batchReader := bufio.NewReader(batchStdoutReader)
+	batchReader := bufio.NewReaderSize(batchStdoutReader, 32*1024)
 
 	return batchStdinWriter, batchReader, cancel
 }
diff --git a/modules/util/remove.go b/modules/util/remove.go
index 754f3b7c1..d05ee9fe4 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
 		}