Prevent dangling cat-files (#17154)

modules/git/batch_reader.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"io"
 	"math"
+	"os"
 	"runtime"
 	"strconv"
 	"strings"
@@ -21,43 +22,44 @@ import (
 	"github.com/djherbis/nio/v3"
 )
 
-// WriteCloserError wraps an io.WriteCloser with an additional CloseWithError function
-type WriteCloserError interface {
-	io.WriteCloser
-	CloseWithError(err error) error
-}
-
 // CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
-func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
-	batchStdinReader, batchStdinWriter := io.Pipe()
+func CatFileBatchCheck(repoPath string) (io.WriteCloser, *bufio.Reader, func()) {
+	batchStdinReader, batchStdinWriter, err := os.Pipe()
+	if err != nil {
+		log.Critical("Unable to open pipe for cat-file --batch: %v", err)
+		rd, wr := io.Pipe()
+		_ = rd.CloseWithError(err)
+		return wr, bufio.NewReader(rd), nil
+	}
 	batchStdoutReader, batchStdoutWriter := io.Pipe()
 	ctx, ctxCancel := context.WithCancel(DefaultContext)
-	closed := make(chan struct{})
+
+	_, filename, line, _ := runtime.Caller(2)
+	filename = strings.TrimPrefix(filename, callerPrefix)
+	desc := fmt.Sprintf("%s cat-file --batch-check [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)
+	cmd := NewCommandContext(ctx, "cat-file", "--batch-check").
+		SetDescription(desc)
+
 	cancel := func() {
+		ctxCancel()
 		_ = batchStdinReader.Close()
 		_ = batchStdinWriter.Close()
 		_ = batchStdoutReader.Close()
 		_ = batchStdoutWriter.Close()
-		ctxCancel()
-		<-closed
 	}
 
-	_, filename, line, _ := runtime.Caller(2)
-	filename = strings.TrimPrefix(filename, callerPrefix)
-
 	go func() {
+		defer ctxCancel()
+
 		stderr := strings.Builder{}
-		err := NewCommandContext(ctx, "cat-file", "--batch-check").
-			SetDescription(fmt.Sprintf("%s cat-file --batch-check [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)).
+		err := cmd.
 			RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
 		if err != nil {
 			_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
-			_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
-		} else {
-			_ = batchStdoutWriter.Close()
-			_ = batchStdinReader.Close()
 		}
-		close(closed)
+
+		_ = batchStdoutWriter.Close()
+		_ = batchStdinReader.Close()
 	}()
 
 	// For simplicities sake we'll use a buffered reader to read from the cat-file --batch-check
@@ -67,38 +69,44 @@ func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()
 }
 
 // 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) (WriteCloserError, *bufio.Reader, func()) {
+func CatFileBatch(repoPath string) (io.WriteCloser, *bufio.Reader, func()) {
 	// 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()
+	batchStdinReader, batchStdinWriter, err := os.Pipe()
+	if err != nil {
+		log.Critical("Unable to open pipe for cat-file --batch: %v", err)
+		rd, wr := io.Pipe()
+		_ = rd.CloseWithError(err)
+		return wr, bufio.NewReader(rd), nil
+	}
+
+	_, filename, line, _ := runtime.Caller(2)
+	filename = strings.TrimPrefix(filename, callerPrefix)
+
+	desc := fmt.Sprintf("%s cat-file --batch [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)
+
 	batchStdoutReader, batchStdoutWriter := nio.Pipe(buffer.New(32 * 1024))
 	ctx, ctxCancel := context.WithCancel(DefaultContext)
-	closed := make(chan struct{})
+	cmd := NewCommandContext(ctx, "cat-file", "--batch").
+		SetDescription(desc)
+
 	cancel := func() {
-		_ = batchStdinReader.Close()
+		ctxCancel()
 		_ = batchStdinWriter.Close()
 		_ = batchStdoutReader.Close()
-		_ = batchStdoutWriter.Close()
-		ctxCancel()
-		<-closed
 	}
 
-	_, filename, line, _ := runtime.Caller(2)
-	filename = strings.TrimPrefix(filename, callerPrefix)
-
 	go func() {
+		defer ctxCancel()
+
 		stderr := strings.Builder{}
-		err := NewCommandContext(ctx, "cat-file", "--batch").
-			SetDescription(fmt.Sprintf("%s cat-file --batch [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)).
-			RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
+		err := cmd.RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
 		if err != nil {
 			_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
-			_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
-		} else {
-			_ = batchStdoutWriter.Close()
-			_ = batchStdinReader.Close()
 		}
-		close(closed)
+
+		_ = batchStdoutWriter.Close()
+		_ = batchStdinReader.Close()
 	}()
 
 	// For simplicities sake we'll us a buffered reader to read from the cat-file --batch

modules/git/command.go

@@ -145,14 +145,14 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
 	cmd.Stdin = stdin
-	if err := cmd.Start(); err != nil {
-		return err
-	}
-
 	desc := c.desc
 	if desc == "" {
 		desc = fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir)
 	}
+	if err := cmd.Start(); err != nil {
+		return err
+	}
+
 	pid := process.GetManager().Add(desc, cancel)
 	defer process.GetManager().Remove(pid)
 

modules/git/last_commit_cache_nogogit.go

@@ -9,6 +9,7 @@ package git
 import (
 	"bufio"
 	"context"
+	"io"
 	"path"
 
 	"code.gitea.io/gitea/modules/log"
@@ -38,7 +39,7 @@ func NewLastCommitCache(repoPath string, gitRepo *Repository, ttl func() int64,
 }
 
 // Get get the last commit information by commit id and entry path
-func (c *LastCommitCache) Get(ref, entryPath string, wr WriteCloserError, rd *bufio.Reader) (interface{}, error) {
+func (c *LastCommitCache) Get(ref, entryPath string, wr io.WriteCloser, rd *bufio.Reader) (interface{}, error) {
 	v := c.cache.Get(c.getCacheKey(c.repoPath, ref, entryPath))
 	if vs, ok := v.(string); ok {
 		log.Debug("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, vs)

modules/git/repo_base_nogogit.go

@@ -11,6 +11,7 @@ import (
 	"bufio"
 	"context"
 	"errors"
+	"io"
 	"path/filepath"
 
 	"code.gitea.io/gitea/modules/log"
@@ -26,11 +27,11 @@ type Repository struct {
 
 	batchCancel context.CancelFunc
 	batchReader *bufio.Reader
-	batchWriter WriteCloserError
+	batchWriter io.WriteCloser
 
 	checkCancel context.CancelFunc
 	checkReader *bufio.Reader
-	checkWriter WriteCloserError
+	checkWriter io.WriteCloser
 }
 
 // OpenRepository opens the repository at the given path.
@@ -54,7 +55,7 @@ func OpenRepository(repoPath string) (*Repository, error) {
 }
 
 // CatFileBatch obtains a CatFileBatch for this repository
-func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func()) {
+func (repo *Repository) CatFileBatch() (io.WriteCloser, *bufio.Reader, func()) {
 	if repo.batchCancel == nil || repo.batchReader.Buffered() > 0 {
 		log.Debug("Opening temporary cat file batch for: %s", repo.Path)
 		return CatFileBatch(repo.Path)
@@ -63,7 +64,7 @@ func (repo *Repository) CatFileBatch() (WriteCloserError, *bufio.Reader, func())
 }
 
 // CatFileBatchCheck obtains a CatFileBatchCheck for this repository
-func (repo *Repository) CatFileBatchCheck() (WriteCloserError, *bufio.Reader, func()) {
+func (repo *Repository) CatFileBatchCheck() (io.WriteCloser, *bufio.Reader, func()) {
 	if repo.checkCancel == nil || repo.checkReader.Buffered() > 0 {
 		log.Debug("Opening temporary cat file batch-check: %s", repo.Path)
 		return CatFileBatchCheck(repo.Path)

modules/indexer/code/bleve.go

@@ -177,7 +177,7 @@ func NewBleveIndexer(indexDir string) (*BleveIndexer, bool, error) {
 	return indexer, created, err
 }
 
-func (b *BleveIndexer) addUpdate(batchWriter git.WriteCloserError, batchReader *bufio.Reader, commitSha string,
+func (b *BleveIndexer) addUpdate(batchWriter io.WriteCloser, batchReader *bufio.Reader, commitSha string,
 	update fileUpdate, repo *models.Repository, batch *gitea_bleve.FlushingBatch) error {
 	// Ignore vendored files in code search
 	if setting.Indexer.ExcludeVendored && analyze.IsVendor(update.Filename) {

modules/indexer/code/elastic_search.go

@@ -175,7 +175,7 @@ func (b *ElasticSearchIndexer) init() (bool, error) {
 	return exists, nil
 }
 
-func (b *ElasticSearchIndexer) addUpdate(batchWriter git.WriteCloserError, batchReader *bufio.Reader, sha string, update fileUpdate, repo *models.Repository) ([]elastic.BulkableRequest, error) {
+func (b *ElasticSearchIndexer) addUpdate(batchWriter io.WriteCloser, batchReader *bufio.Reader, sha string, update fileUpdate, repo *models.Repository) ([]elastic.BulkableRequest, error) {
 	// Ignore vendored files in code search
 	if setting.Indexer.ExcludeVendored && analyze.IsVendor(update.Filename) {
 		return nil, nil