~sircmpwn/annotatego

02e9b7f8826bcd7713c126c47d99fce51635ba4b — Chris Waldon 2 years ago e52c42d
bugfix: eliminate race reading from git ls-tree

This commit replaces the use of exec.Cmd.Run() with the use of both
exec.Cmd.Start() and exec.Cmd.Wait(). This is because Run() can close
the stdout pipe prematurely, and should not be used at the same time as
exec.Cmd.StdoutPipe() as per the docs here:

https://godoc.org/os/exec#Cmd.StdoutPipe

The result was that sometimes not all of the output of git ls-tree would
be parsed, and this led to many files which were actually present in the
git tree being ignored by the annotator. When this race is triggered,
the annotator usually provides no annotations whatsoever.

For an example of this race being triggered in the wild, see here:

https://builds.sr.ht/~eliasnaur/job/168932#task-annotate-57
1 files changed, 6 insertions(+), 5 deletions(-)

M main.go
M main.go => main.go +6 -5
@@ 67,11 67,9 @@ func crawlGitTree() map[string]string {
	}
	defer pipe.Close()
	scanner := bufio.NewScanner(pipe)
	go func() {
		if err := cmd.Run(); err != nil {
			panic(err)
		}
	}()
	if err := cmd.Run(); err != nil {
		panic(err)
	}
	scanner.Split(scanNulls)
	m := make(map[string]string)
	for scanner.Scan() {


@@ 85,6 83,9 @@ func crawlGitTree() map[string]string {
		path := strings.Join(fields[3:], " ")
		m[path] = oid
	}
	if err := cmd.Wait(); err != nil {
		panic(err)
	}
	return m
}