Skip to content

Commit be95ba5

Browse files
committed
Prevent intermittent race in attribute reader close (go-gitea#19537)
Backport go-gitea#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 <[email protected]>
1 parent 1465e0c commit be95ba5

File tree

1 file changed

+2
-16
lines changed

1 file changed

+2
-16
lines changed

modules/git/repo_attribute.go

+2-16
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,10 @@ type CheckAttributeReader struct {
119119
env []string
120120
ctx context.Context
121121
cancel context.CancelFunc
122-
running chan struct{}
123122
}
124123

125124
// Init initializes the cmd
126125
func (c *CheckAttributeReader) Init(ctx context.Context) error {
127-
c.running = make(chan struct{})
128126
cmdArgs := []string{"check-attr", "--stdin", "-z"}
129127

130128
if len(c.IndexFile) > 0 && CheckGitVersionAtLeast("1.7.8") == nil {
@@ -183,14 +181,7 @@ func (c *CheckAttributeReader) Run() error {
183181
_ = c.stdOut.Close()
184182
}()
185183
stdErr := new(bytes.Buffer)
186-
err := c.cmd.RunInDirTimeoutEnvFullPipelineFunc(c.env, -1, c.Repo.Path, c.stdOut, stdErr, c.stdinReader, func(_ context.Context, _ context.CancelFunc) error {
187-
select {
188-
case <-c.running:
189-
default:
190-
close(c.running)
191-
}
192-
return nil
193-
})
184+
err := c.cmd.RunInDirTimeoutEnvFullPipeline(c.env, -1, c.Repo.Path, c.stdOut, stdErr, c.stdinReader)
194185
if err != nil && // If there is an error we need to return but:
195186
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)
196187
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
210201
select {
211202
case <-c.ctx.Done():
212203
return nil, c.ctx.Err()
213-
case <-c.running:
204+
default:
214205
}
215206

216207
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
237228
func (c *CheckAttributeReader) Close() error {
238229
c.cancel()
239230
err := c.stdinWriter.Close()
240-
select {
241-
case <-c.running:
242-
default:
243-
close(c.running)
244-
}
245231
return err
246232
}
247233

0 commit comments

Comments
 (0)