Skip to content

Commit 7ebc3da

Browse files
authored
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 <[email protected]> * move close into the defer Signed-off-by: Andrew Thornton <[email protected]> * lets try again Signed-off-by: Andrew Thornton <[email protected]>
1 parent 2e36ba0 commit 7ebc3da

File tree

3 files changed

+25
-17
lines changed

3 files changed

+25
-17
lines changed

modules/git/repo_attribute.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[
8787
return nil, fmt.Errorf("wrong number of fields in return from check-attr")
8888
}
8989

90-
var name2attribute2info = make(map[string]map[string]string)
90+
name2attribute2info := make(map[string]map[string]string)
9191

9292
for i := 0; i < (len(fields) / 3); i++ {
9393
filename := string(fields[3*i])
@@ -179,17 +179,21 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error {
179179
// Run run cmd
180180
func (c *CheckAttributeReader) Run() error {
181181
defer func() {
182-
_ = c.Close()
182+
_ = c.stdinReader.Close()
183+
_ = c.stdOut.Close()
183184
}()
184185
stdErr := new(bytes.Buffer)
185186
err := c.cmd.RunInDirTimeoutEnvFullPipelineFunc(c.env, -1, c.Repo.Path, c.stdOut, stdErr, c.stdinReader, func(_ context.Context, _ context.CancelFunc) error {
186-
close(c.running)
187+
select {
188+
case <-c.running:
189+
default:
190+
close(c.running)
191+
}
187192
return nil
188193
})
189194
if err != nil && c.ctx.Err() != nil && err.Error() != "signal: killed" {
190195
return fmt.Errorf("failed to run attr-check. Error: %w\nStderr: %s", err, stdErr.String())
191196
}
192-
193197
return nil
194198
}
195199

@@ -229,10 +233,8 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
229233

230234
// Close close pip after use
231235
func (c *CheckAttributeReader) Close() error {
232-
err := c.stdinWriter.Close()
233-
_ = c.stdinReader.Close()
234-
_ = c.stdOut.Close()
235236
c.cancel()
237+
err := c.stdinWriter.Close()
236238
select {
237239
case <-c.running:
238240
default:

modules/git/repo_language_stats_nogogit.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,10 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
8888
}
8989
}()
9090
}
91-
defer cancel()
91+
defer func() {
92+
_ = checker.Close()
93+
cancel()
94+
}()
9295
}
9396
}
9497

services/gitdiff/gitdiff.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,11 @@ var (
190190
codeTagSuffix = []byte(`</span>`)
191191
)
192192

193-
var unfinishedtagRegex = regexp.MustCompile(`<[^>]*$`)
194-
var trailingSpanRegex = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`)
195-
var entityRegex = regexp.MustCompile(`&[#]*?[0-9[:alpha:]]*$`)
193+
var (
194+
unfinishedtagRegex = regexp.MustCompile(`<[^>]*$`)
195+
trailingSpanRegex = regexp.MustCompile(`<span\s*[[:alpha:]="]*?[>]?$`)
196+
entityRegex = regexp.MustCompile(`&[#]*?[0-9[:alpha:]]*$`)
197+
)
196198

197199
// shouldWriteInline represents combinations where we manually write inline changes
198200
func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool {
@@ -206,7 +208,6 @@ func shouldWriteInline(diff diffmatchpatch.Diff, lineType DiffLineType) bool {
206208
}
207209

208210
func fixupBrokenSpans(diffs []diffmatchpatch.Diff) []diffmatchpatch.Diff {
209-
210211
// Create a new array to store our fixed up blocks
211212
fixedup := make([]diffmatchpatch.Diff, 0, len(diffs))
212213

@@ -658,10 +659,10 @@ func (diffFile *DiffFile) GetTailSection(gitRepo *git.Repository, leftCommitID,
658659
LastRightIdx: lastLine.RightIdx,
659660
LeftIdx: leftLineCount,
660661
RightIdx: rightLineCount,
661-
}}
662+
},
663+
}
662664
tailSection := &DiffSection{FileName: diffFile.Name, Lines: []*DiffLine{tailDiffLine}}
663665
return tailSection
664-
665666
}
666667

667668
func getCommitFileLineCount(commit *git.Commit, filePath string) int {
@@ -942,8 +943,8 @@ parsingLoop:
942943
// TODO: There are numerous issues with this:
943944
// - we might want to consider detecting encoding while parsing but...
944945
// - we're likely to fail to get the correct encoding here anyway as we won't have enough information
945-
var diffLineTypeBuffers = make(map[DiffLineType]*bytes.Buffer, 3)
946-
var diffLineTypeDecoders = make(map[DiffLineType]*encoding.Decoder, 3)
946+
diffLineTypeBuffers := make(map[DiffLineType]*bytes.Buffer, 3)
947+
diffLineTypeDecoders := make(map[DiffLineType]*encoding.Decoder, 3)
947948
diffLineTypeBuffers[DiffLinePlain] = new(bytes.Buffer)
948949
diffLineTypeBuffers[DiffLineAdd] = new(bytes.Buffer)
949950
diffLineTypeBuffers[DiffLineDel] = new(bytes.Buffer)
@@ -1420,6 +1421,7 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff
14201421
}()
14211422
}
14221423
defer func() {
1424+
_ = checker.Close()
14231425
cancel()
14241426
}()
14251427
}
@@ -1539,7 +1541,8 @@ func GetWhitespaceFlag(whiteSpaceBehavior string) string {
15391541
"ignore-all": "-w",
15401542
"ignore-change": "-b",
15411543
"ignore-eol": "--ignore-space-at-eol",
1542-
"": ""}
1544+
"": "",
1545+
}
15431546

15441547
return whitespaceFlags[whiteSpaceBehavior]
15451548
}

0 commit comments

Comments
 (0)