plumbing: ssh, Fix flaky test TestAdvertisedReferencesNotExists. Fixes #969

[mccurdyc]

Proposed Changes

• Fix Flaky test TestAdvertisedReferencesNotExists #969
• Explicitly force exec.Command (git-upload-pack) to wait for Stdout and Stderr to finish copying before closing them with cmd.Wait
• removed the unnecessary stderr.Close() and stdout.Close()
  • https://golang.org/pkg/os/exec/#Cmd.StderrPipe
  • https://golang.org/pkg/os/exec/#Cmd.StdoutPipe

  Wait will close the pipe after seeing the command exit, so most callers need not close the pipe themselves

Testing Methodology

• I wrote a script

#!/usr/bin/env bash
for i in {1..5000}
do
  echo $i

  go test -v &>> out.txt

  if grep -q 'FAIL' out.txt
  then
    break
  fi
done

Previously, using this script and running 4 instances would cause the test to fail after about 50 iterations (i.e., the race condition would occur). It has run 850 iterations across all four instances without a failing test case.

Note: running 4 instances isn't necessary, but it does constrain resources and probably helps trigger the race condition

Relevant/Related Links

• os/exec: data race between StdoutPipe and Wait golang/go#19685
• https://hackmongo.com/post/reading-os-exec-cmd-output-without-race-conditions/
• https://hk.saowen.com/a/70926bb4d233965fc2fcefa33abd97f632489a21fffb2ee6270ab210c3bf9321
• https://golang.org/pkg/os/exec/#Cmd.Wait