diff options
author | Alberto Cortés <alcortesm@gmail.com> | 2017-02-15 12:41:02 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-02-15 12:41:02 +0100 |
commit | f5a9c7ed5ab58afa255c2f405b30f06f88a6aecf (patch) | |
tree | b334e2b42560fac69469b352329b8527adab7ab6 /plumbing/storer/storer.go | |
parent | 48fcea26a90a0e3afea13a23fa4e5e9f6a910f0b (diff) | |
download | go-git-f5a9c7ed5ab58afa255c2f405b30f06f88a6aecf.tar.gz |
transport/file: fix race condition on test (#267)
Sometimes, the `TestClone` and `TestPush` tests of `transport/file` fail
in travis.
This is due to a race condition caused by an incorrect usage of
`Cmd.Wait()` while reading from the output and error pipes of the
command.
This patch fixes the problem by using `Cmd.CombinedOutput()` instead of
calling `Cmd.Start()` and `Cmd.Wait()` while reading from the output and
error pipes.
Details:
From the `exec` package documentation:
```
Wait will close the pipe after seeing the command exit, so most callers
need not close the pipe themselves; however, an implication is that it
is incorrect to call Wait before all reads from the pipe have completed.
For the same reason, it is incorrect to call Run when using StdoutPipe.
```
In our tests, the old `execAndGetOutput` function was creating two
gorutines to read from the stderr and stdout pipes of the command and
then call `Wait` on the command.
This caused a race condition: when the `Wait` call finished before the
gorutines have read from the pipes, they returned caused an error on
`outErr`.
The problem only happens sometimes on travis.
To reproduce the problem locally, just add a call to
time.Sleep(time.Second) to the gorutine before its `ioutil.ReadAll` call
to delay them, then `Wait` will always finish before them, closing the
pipes, and the gorutines will fail. The returned error detected by the
test will be:
```
FAIL: server_test.go:55: ServerSuite.TestClone
server_test.go:65:
c.Assert(err, IsNil, Commentf("STDOUT:\n%s\nSTDERR:\n%s\n", stdout, stderr))
... value *os.PathError = &os.PathError{Op:"read", Path:"|0", Err:0x9} ("read |0: bad file descriptor")
... STDOUT:
STDERR:
```
Diffstat (limited to 'plumbing/storer/storer.go')
0 files changed, 0 insertions, 0 deletions