Prevent intermittent race in attribute reader close (#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>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: 6543 <6543@obermui.de>
pull/19448/head^2
zeripath 3 years ago committed by GitHub
parent ca4b920bbc
commit 332b2ecd21
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 17
      modules/git/repo_attribute.go

@ -124,12 +124,10 @@ type CheckAttributeReader struct {
env []string env []string
ctx context.Context ctx context.Context
cancel context.CancelFunc cancel context.CancelFunc
running chan struct{}
} }
// Init initializes the cmd // Init initializes the cmd
func (c *CheckAttributeReader) Init(ctx context.Context) error { func (c *CheckAttributeReader) Init(ctx context.Context) error {
c.running = make(chan struct{})
cmdArgs := []string{"check-attr", "--stdin", "-z"} cmdArgs := []string{"check-attr", "--stdin", "-z"}
if len(c.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil { if len(c.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil {
@ -194,14 +192,6 @@ func (c *CheckAttributeReader) Run() error {
Stdin: c.stdinReader, Stdin: c.stdinReader,
Stdout: c.stdOut, Stdout: c.stdOut,
Stderr: stdErr, Stderr: stdErr,
PipelineFunc: func(_ context.Context, _ context.CancelFunc) error {
select {
case <-c.running:
default:
close(c.running)
}
return nil
},
}) })
if err != nil && // If there is an error we need to return but: 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) 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)
@ -222,7 +212,7 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
select { select {
case <-c.ctx.Done(): case <-c.ctx.Done():
return nil, c.ctx.Err() return nil, c.ctx.Err()
case <-c.running: default:
} }
if _, err = c.stdinWriter.Write([]byte(path + "\x00")); err != nil { if _, err = c.stdinWriter.Write([]byte(path + "\x00")); err != nil {
@ -249,11 +239,6 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
func (c *CheckAttributeReader) Close() error { func (c *CheckAttributeReader) Close() error {
c.cancel() c.cancel()
err := c.stdinWriter.Close() err := c.stdinWriter.Close()
select {
case <-c.running:
default:
close(c.running)
}
return err return err
} }

Loading…
Cancel
Save