Prevent intermittent race in attribute reader close (#19537) (#19539)

Backport #19537

There is a potential rare race possible whereby the c.running channel could
be closed twice. Looking at the code I do not see a need for this c.running
channel and therefore I think we can remove this. (I think the c.running
might have been some attempt to prevent a hang but the use of os.Pipes should
prevent that.)

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

diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go
index c107308fd6358ff268aad830474ed60280c60f7f..1fbedae5d5d633ce1a6129eef69e286b4ac219a4 100644 (file)
--- a/modules/git/repo_attribute.go
+++ b/modules/git/repo_attribute.go
@@ -119,12 +119,10 @@ type CheckAttributeReader struct {
        env         []string
        ctx         context.Context
        cancel      context.CancelFunc
-       running     chan struct{}
 }
 
 // Init initializes the cmd
 func (c *CheckAttributeReader) Init(ctx context.Context) error {
-       c.running = make(chan struct{})
        cmdArgs := []string{"check-attr", "--stdin", "-z"}
 
        if len(c.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil {
@@ -183,14 +181,7 @@ func (c *CheckAttributeReader) Run() error {
                _ = 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 {
-               select {
-               case <-c.running:
-               default:
-                       close(c.running)
-               }
-               return nil
-       })
+       err := c.cmd.RunInDirTimeoutEnvFullPipeline(c.env, -1, c.Repo.Path, c.stdOut, stdErr, c.stdinReader)
        if err != nil && //                      If there is an error we need to return but:
                c.ctx.Err() != err && //             1. Ignore the context error if the context is cancelled or exceeds the deadline (RunWithContext could return c.ctx.Err() which is Canceled or DeadlineExceeded)
                err.Error() != "signal: killed" { // 2. We should not pass up errors due to the program being killed
@@ -210,7 +201,7 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
        select {
        case <-c.ctx.Done():
                return nil, c.ctx.Err()
-       case <-c.running:
+       default:
        }
 
        if _, err = c.stdinWriter.Write([]byte(path + "\x00")); err != nil {
@@ -237,11 +228,6 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
 func (c *CheckAttributeReader) Close() error {
        c.cancel()
        err := c.stdinWriter.Close()
-       select {
-       case <-c.running:
-       default:
-               close(c.running)
-       }
        return err
 }