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 <art27@cantab.net>

* move close into the defer

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

* lets try again

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2022-02-14 20:27:55 +00:00 committed by GitHub
parent 2e36ba0a00
commit 7ebc3da7cb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 25 additions and 17 deletions

View file

@ -87,7 +87,7 @@ func (repo *Repository) CheckAttribute(opts CheckAttributeOpts) (map[string]map[
return nil, fmt.Errorf("wrong number of fields in return from check-attr")
}
var name2attribute2info = make(map[string]map[string]string)
name2attribute2info := make(map[string]map[string]string)
for i := 0; i < (len(fields) / 3); i++ {
filename := string(fields[3*i])
@ -179,17 +179,21 @@ func (c *CheckAttributeReader) Init(ctx context.Context) error {
// Run run cmd
func (c *CheckAttributeReader) Run() error {
defer func() {
_ = c.Close()
_ = c.stdinReader.Close()
_ = 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 {
close(c.running)
select {
case <-c.running:
default:
close(c.running)
}
return nil
})
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
}
@ -229,10 +233,8 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err
// Close close pip after use
func (c *CheckAttributeReader) Close() error {
err := c.stdinWriter.Close()
_ = c.stdinReader.Close()
_ = c.stdOut.Close()
c.cancel()
err := c.stdinWriter.Close()
select {
case <-c.running:
default: