Prevent dangling GetAttribute calls (#18754) (#18755)

* Prevent dangling GetAttribute calls

It appears possible that there could be a hang due to unread data from the
repo-attribute command pipes. This PR simply closes these during the defer.

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

* move close into the defer

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

* lets try again

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

3 files changed, 25 insertions, 17 deletions

diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go
index 0bb550bb4b..3f6f8943a9 100644
--- a/modules/git/repo_attribute.go
+++ b/modules/git/repo_attribute.go
@@ -87,7 +87,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[
 		return nil, fmt.Errorf("wrong number of fields in return from check-attr")
 	}
 
-	var name2attribute2info = make(map[string]map[string]string)
+	name2attribute2info := make(map[string]map[string]string)
 
 	for i := 0; i < (len(fields) / 3); i++ {
 		filename := string(fields[3*i])
@@ -179,17 +179,21 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error {
 // Run run cmd
 func (c *CheckAttributeReader) Run() error {
 	defer func() {
-		_ = c.Close()
+		_ = c.stdinReader.Close()
+		_ = c.stdOut.Close()
 	}()
 	stdErr := new(bytes.Buffer)
 	err := c.cmd.RunInDirTimeoutEnvFullPipelineFunc(c.env, -1, c.Repo.Path, c.stdOut, stdErr, c.stdinReader, func(_ context.Context, _ context.CancelFunc) error {
-		close(c.running)
+		select {
+		case <-c.running:
+		default:
+			close(c.running)
+		}
 		return nil
 	})
 	if err != nil && c.ctx.Err() != nil && err.Error() != "signal: killed" {
 		return fmt.Errorf("failed to run attr-check. Error: %w\nStderr: %s", err, stdErr.String())
 	}
-
 	return nil
 }
 
@@ -229,10 +233,8 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
 
 // Close close pip after use
 func (c *CheckAttributeReader) Close() error {
-	err := c.stdinWriter.Close()
-	_ = c.stdinReader.Close()
-	_ = c.stdOut.Close()
 	c.cancel()
+	err := c.stdinWriter.Close()
 	select {
 	case <-c.running:
 	default:
diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go
index 0b21bf6344..adb11dd8fa 100644
--- a/modules/git/repo_language_stats_nogogit.go
+++ b/modules/git/repo_language_stats_nogogit.go
@@ -88,7 +88,10 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
 					}
 				}()
 			}
-			defer cancel()
+			defer func() {
+				_ = checker.Close()
+				cancel()
+			}()
 		}
 	}
 
diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go
index fb3e32935d..28c034fafd 100644
--- a/services/gitdiff/gitdiff.go
+++ b/services/gitdiff/gitdiff.go
@@ -190,9 +190,11 @@ var (
 	codeTagSuffix     = []byte(`</span>`)
 )
 
-var unfinishedtagRegex = regexp.MustCompile(`<[^>]*$`)
-var trailingSpanRegex = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`)
-var entityRegex = regexp.MustCompile(`&[#]*?[0-9[:alpha:]]*$`)
+var (
+	unfinishedtagRegex = regexp.MustCompile(`<[^>]*$`)
+	trailingSpanRegex  = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`)
+	entityRegex        = regexp.MustCompile(`&[#]*?[0-9[:alpha:]]*$`)
+)
 
 // shouldWriteInline represents combinations where we manually write inline changes
 func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool {
@@ -206,7 +208,6 @@ func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool {
 }
 
 func fixupBrokenSpans(diffs []diffmatchpatch.Diff) []diffmatchpatch.Diff {
-
 	// Create a new array to store our fixed up blocks
 	fixedup := make([]diffmatchpatch.Diff, 0, len(diffs))
 
@@ -658,10 +659,10 @@ func (diffFile *DiffFile) GetTailSection(gitRepo *git.Repository, leftCommitID,
 			LastRightIdx: lastLine.RightIdx,
 			LeftIdx:      leftLineCount,
 			RightIdx:     rightLineCount,
-		}}
+		},
+	}
 	tailSection := &DiffSection{FileName: diffFile.Name, Lines: []*DiffLine{tailDiffLine}}
 	return tailSection
-
 }
 
 func getCommitFileLineCount(commit *git.Commit, filePath string) int {
@@ -942,8 +943,8 @@ parsingLoop:
 	// TODO: There are numerous issues with this:
 	// - we might want to consider detecting encoding while parsing but...
 	// - we're likely to fail to get the correct encoding here anyway as we won't have enough information
-	var diffLineTypeBuffers = make(map[DiffLineType]*bytes.Buffer, 3)
-	var diffLineTypeDecoders = make(map[DiffLineType]*encoding.Decoder, 3)
+	diffLineTypeBuffers := make(map[DiffLineType]*bytes.Buffer, 3)
+	diffLineTypeDecoders := make(map[DiffLineType]*encoding.Decoder, 3)
 	diffLineTypeBuffers[DiffLinePlain] = new(bytes.Buffer)
 	diffLineTypeBuffers[DiffLineAdd] = new(bytes.Buffer)
 	diffLineTypeBuffers[DiffLineDel] = new(bytes.Buffer)
@@ -1420,6 +1421,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
 				}()
 			}
 			defer func() {
+				_ = checker.Close()
 				cancel()
 			}()
 		}
@@ -1539,7 +1541,8 @@ func GetWhitespaceFlag(whiteSpaceBehavior string) string {
 		"ignore-all":    "-w",
 		"ignore-change": "-b",
 		"ignore-eol":    "--ignore-space-at-eol",
-		"":              ""}
+		"":              "",
+	}
 
 	return whitespaceFlags[whiteSpaceBehavior]
 }