From 32342b3f3075aed7a2cdb672862c9c96f267fe58 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 13 Feb 2022 21:09:03 +0000 Subject: [PATCH 1/3] 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 --- services/gitdiff/gitdiff.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 017341d63f648..c785e49211da0 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1418,10 +1418,12 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff if err != nil && err != ctx.Err() { log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) } + _ = checker.Close() cancel() }() } defer func() { + _ = checker.Close() cancel() }() } From 3e90e8a56d268b6722a504024df4388746174102 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 14 Feb 2022 13:10:45 +0000 Subject: [PATCH 2/3] move close into the defer Signed-off-by: Andrew Thornton --- modules/git/repo_attribute.go | 11 ++++++----- modules/git/repo_language_stats_nogogit.go | 5 ++++- services/gitdiff/gitdiff.go | 1 - 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index d31203aabd049..a6d4dd59f7945 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -184,9 +184,6 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error { // Run run cmd func (c *CheckAttributeReader) Run() error { - defer func() { - _ = c.Close() - }() stdErr := new(bytes.Buffer) err := c.cmd.RunWithContext(&RunContext{ Env: c.env, @@ -196,7 +193,11 @@ func (c *CheckAttributeReader) Run() error { Stdout: c.stdOut, Stderr: stdErr, PipelineFunc: func(_ context.Context, _ context.CancelFunc) error { - close(c.running) + select { + case <-c.running: + default: + close(c.running) + } return nil }, }) @@ -243,10 +244,10 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err // Close close pip after use func (c *CheckAttributeReader) Close() error { + c.cancel() err := c.stdinWriter.Close() _ = c.stdinReader.Close() _ = c.stdOut.Close() - c.cancel() select { case <-c.running: default: diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 0b21bf6344c1c..adb11dd8fa6a0 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 c785e49211da0..58c25ff98f827 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1418,7 +1418,6 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff if err != nil && err != ctx.Err() { log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err) } - _ = checker.Close() cancel() }() } From 3b13959587f53aada03d3e0656548bccb307602c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 14 Feb 2022 16:04:30 +0000 Subject: [PATCH 3/3] lets try again Signed-off-by: Andrew Thornton --- modules/git/repo_attribute.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index a6d4dd59f7945..772ee6ad12b46 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -184,6 +184,10 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error { // Run run cmd func (c *CheckAttributeReader) Run() error { + defer func() { + _ = c.stdinReader.Close() + _ = c.stdOut.Close() + }() stdErr := new(bytes.Buffer) err := c.cmd.RunWithContext(&RunContext{ Env: c.env, @@ -204,7 +208,6 @@ func (c *CheckAttributeReader) Run() error { 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 } @@ -246,8 +249,6 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err func (c *CheckAttributeReader) Close() error { c.cancel() err := c.stdinWriter.Close() - _ = c.stdinReader.Close() - _ = c.stdOut.Close() select { case <-c.running: default: