Add missing cmd.Close in log.GetCommit

34 files changed, 170 insertions, 74 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index df545c268..0aa2ee64c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@ UNRELEASED
 
 - Update vendor/gitlab_git to b58c4f436abaf646703bdd80f266fa4c0bab2dd2
   https://gitlab.com/gitlab-org/gitaly/merge_requests/324
+- Add missing cmd.Close in log.GetCommit
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/326
 
 v0.37.0
 
diff --git a/internal/helper/command.go b/internal/command/command.go
index cc4ed735c..21aa5fec9 100644
--- a/internal/helper/command.go
+++ b/internal/command/command.go
@@ -1,4 +1,4 @@
-package helper
+package command
 
 import (
 	"context"
@@ -61,23 +61,23 @@ func GitPath() string {
 	return config.Config.Git.BinPath
 }
 
-// GitCommandReader creates a git Command with the given args
-func GitCommandReader(ctx context.Context, args ...string) (*Command, error) {
-	return NewCommand(ctx, exec.Command(GitPath(), args...), nil, nil, nil)
+// Git creates a git Command with the given args
+func Git(ctx context.Context, args ...string) (*Command, error) {
+	return New(ctx, exec.Command(GitPath(), args...), nil, nil, nil)
 }
 
-// GitlabShellCommandReader creates a gitlab-shell Command with the given args
-func GitlabShellCommandReader(ctx context.Context, envs []string, executable string, args ...string) (*Command, error) {
+// GitlabShell creates a gitlab-shell Command with the given args
+func GitlabShell(ctx context.Context, envs []string, executable string, args ...string) (*Command, error) {
 	shellPath, ok := config.GitlabShellPath()
 	if !ok {
 		return nil, fmt.Errorf("path to gitlab-shell not set")
 	}
 	// Don't allow any git-command to ask (interactively) for credentials
-	return NewCommand(ctx, exec.Command(path.Join(shellPath, executable), args...), nil, nil, nil, envs...)
+	return New(ctx, exec.Command(path.Join(shellPath, executable), args...), nil, nil, nil, envs...)
 }
 
-// NewCommand creates a Command from an exec.Cmd
-func NewCommand(ctx context.Context, cmd *exec.Cmd, stdin io.Reader, stdout, stderr io.Writer, env ...string) (*Command, error) {
+// New creates a Command from an exec.Cmd
+func New(ctx context.Context, cmd *exec.Cmd, stdin io.Reader, stdout, stderr io.Writer, env ...string) (*Command, error) {
 	grpc_logrus.Extract(ctx).WithFields(log.Fields{
 		"path": cmd.Path,
 		"args": cmd.Args,
diff --git a/internal/helper/command_test.go b/internal/command/command_test.go
index 25ddc7491..c24779eac 100644
--- a/internal/helper/command_test.go
+++ b/internal/command/command_test.go
@@ -1,4 +1,4 @@
-package helper
+package command
 
 import (
 	"bytes"
@@ -18,7 +18,7 @@ func TestNewCommandTZEnv(t *testing.T) {
 	os.Setenv("TZ", "foobar")
 
 	buff := &bytes.Buffer{}
-	cmd, err := NewCommand(context.Background(), exec.Command("env"), nil, buff, nil)
+	cmd, err := New(context.Background(), exec.Command("env"), nil, buff, nil)
 
 	require.NoError(t, err)
 	require.NoError(t, cmd.Wait())
@@ -29,7 +29,7 @@ func TestNewCommandTZEnv(t *testing.T) {
 func TestNewCommandExtraEnv(t *testing.T) {
 	extraVar := "FOOBAR=123456"
 	buff := &bytes.Buffer{}
-	cmd, err := NewCommand(context.Background(), exec.Command("/usr/bin/env"), nil, buff, nil, extraVar)
+	cmd, err := New(context.Background(), exec.Command("/usr/bin/env"), nil, buff, nil, extraVar)
 
 	require.NoError(t, err)
 	require.NoError(t, cmd.Wait())
diff --git a/internal/git/catfile/catfile.go b/internal/git/catfile/catfile.go
index ac12253da..5fbae763c 100644
--- a/internal/git/catfile/catfile.go
+++ b/internal/git/catfile/catfile.go
@@ -9,7 +9,8 @@ import (
 	"strconv"
 	"strings"
 
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
+
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 )
@@ -31,7 +32,7 @@ type Handler func(io.Writer, *bufio.Reader) error
 func CatFile(ctx context.Context, repoPath string, handler Handler) error {
 	stdinReader, stdinWriter := io.Pipe()
 	cmdArgs := []string{"--git-dir", repoPath, "cat-file", "--batch"}
-	cmd, err := helper.NewCommand(ctx, exec.Command(helper.GitPath(), cmdArgs...), stdinReader, nil, nil)
+	cmd, err := command.New(ctx, exec.Command(command.GitPath(), cmdArgs...), stdinReader, nil, nil)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "CatFile: cmd: %v", err)
 	}
diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go
index a727d76e1..62ec6c65c 100644
--- a/internal/git/log/commit.go
+++ b/internal/git/log/commit.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"strings"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -38,6 +39,7 @@ func GetCommit(ctx context.Context, repo *pb.Repository, revision string, path s
 	if err != nil {
 		return nil, err
 	}
+	defer cmd.Close()
 
 	logParser := NewLogParser(cmd)
 	if ok := logParser.Parse(); !ok {
@@ -48,7 +50,7 @@ func GetCommit(ctx context.Context, repo *pb.Repository, revision string, path s
 }
 
 // GitLogCommand returns a Command that executes git log with the given the arguments
-func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string, paths []string, extraArgs ...string) (*helper.Command, error) {
+func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string, paths []string, extraArgs ...string) (*command.Command, error) {
 	grpc_logrus.Extract(ctx).WithFields(log.Fields{
 		"Revisions": revisions,
 	}).Debug("GitLog")
@@ -72,7 +74,7 @@ func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string,
 	args = append(args, "--")
 	args = append(args, paths...)
 
-	cmd, err := helper.GitCommandReader(ctx, args...)
+	cmd, err := command.Git(ctx, args...)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/helper/repo.go b/internal/helper/repo.go
index 8143c205e..ed392912b 100644
--- a/internal/helper/repo.go
+++ b/internal/helper/repo.go
@@ -6,6 +6,7 @@ import (
 	"path"
 	"strings"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -87,7 +88,7 @@ func IsValidRef(ctx context.Context, path, ref string) bool {
 		return false
 	}
 
-	cmd, err := GitCommandReader(ctx, "--git-dir", path, "log", "--max-count=1", ref)
+	cmd, err := command.Git(ctx, "--git-dir", path, "log", "--max-count=1", ref)
 	if err != nil {
 		return false
 	}
diff --git a/internal/linguist/linguist.go b/internal/linguist/linguist.go
index 497f3b9a3..b8ab27269 100644
--- a/internal/linguist/linguist.go
+++ b/internal/linguist/linguist.go
@@ -11,8 +11,8 @@ import (
 	"path"
 	"strings"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 var (
@@ -28,7 +28,7 @@ type Language struct {
 func Stats(ctx context.Context, repoPath string, commitID string) (map[string]int, error) {
 	cmd := exec.Command("bundle", "exec", "bin/ruby-cd", repoPath, "git-linguist", "--commit="+commitID, "stats")
 	cmd.Dir = config.Config.Ruby.Dir
-	reader, err := helper.NewCommand(ctx, cmd, nil, nil, nil, os.Environ()...)
+	reader, err := command.New(ctx, cmd, nil, nil, nil, os.Environ()...)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go
index a2b02c088..3477bcc47 100644
--- a/internal/rubyserver/rubyserver.go
+++ b/internal/rubyserver/rubyserver.go
@@ -12,6 +12,7 @@ import (
 	"sync"
 	"time"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/supervisor"
@@ -89,7 +90,7 @@ func Start() (*Server, error) {
 	lazyInit.Do(prepareSocketPath)
 
 	args := []string{"bundle", "exec", "bin/gitaly-ruby", fmt.Sprintf("%d", os.Getpid()), socketPath()}
-	env := append(os.Environ(), "GITALY_RUBY_GIT_BIN_PATH="+helper.GitPath(),
+	env := append(os.Environ(), "GITALY_RUBY_GIT_BIN_PATH="+command.GitPath(),
 		fmt.Sprintf("GITALY_RUBY_WRITE_BUFFER_SIZE=%d", streamio.WriteBufferSize))
 	p, err := supervisor.New("gitaly-ruby", env, args, config.Config.Ruby.Dir)
 	return &Server{Process: p}, err
diff --git a/internal/service/blob/get_blob.go b/internal/service/blob/get_blob.go
index 7ebca51d1..2cd1344e5 100644
--- a/internal/service/blob/get_blob.go
+++ b/internal/service/blob/get_blob.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"os/exec"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
@@ -29,7 +30,7 @@ func (s *server) GetBlob(in *pb.GetBlobRequest, stream pb.BlobService_GetBlobSer
 	stdinReader, stdinWriter := io.Pipe()
 
 	cmdArgs := []string{"--git-dir", repoPath, "cat-file", "--batch"}
-	cmd, err := helper.NewCommand(stream.Context(), exec.Command(helper.GitPath(), cmdArgs...), stdinReader, nil, nil)
+	cmd, err := command.New(stream.Context(), exec.Command(command.GitPath(), cmdArgs...), stdinReader, nil, nil)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "GetBlob: cmd: %v", err)
 	}
diff --git a/internal/service/commit/count_commits.go b/internal/service/commit/count_commits.go
index 538fa2f22..6663acb13 100644
--- a/internal/service/commit/count_commits.go
+++ b/internal/service/commit/count_commits.go
@@ -7,6 +7,7 @@ import (
 	"strconv"
 	"time"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -39,7 +40,7 @@ func (s *server) CountCommits(ctx context.Context, in *pb.CountCommitsRequest) (
 		cmdArgs = append(cmdArgs, "--", string(path))
 	}
 
-	cmd, err := helper.GitCommandReader(ctx, cmdArgs...)
+	cmd, err := command.Git(ctx, cmdArgs...)
 	if err != nil {
 		return nil, grpc.Errorf(codes.Internal, "CountCommits: cmd: %v", err)
 	}
diff --git a/internal/service/commit/isancestor.go b/internal/service/commit/isancestor.go
index 49c85f886..53990c900 100644
--- a/internal/service/commit/isancestor.go
+++ b/internal/service/commit/isancestor.go
@@ -13,6 +13,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
@@ -39,8 +40,8 @@ func commitIsAncestorName(ctx context.Context, path, ancestorID, childID string)
 		"childSha":    childID,
 	}).Debug("commitIsAncestor")
 
-	osCommand := exec.Command(helper.GitPath(), "--git-dir", path, "merge-base", "--is-ancestor", ancestorID, childID)
-	cmd, err := helper.NewCommand(ctx, osCommand, nil, ioutil.Discard, nil)
+	osCommand := exec.Command(command.GitPath(), "--git-dir", path, "merge-base", "--is-ancestor", ancestorID, childID)
+	cmd, err := command.New(ctx, osCommand, nil, ioutil.Discard, nil)
 	if err != nil {
 		return false, grpc.Errorf(codes.Internal, err.Error())
 	}
diff --git a/internal/service/commit/languages.go b/internal/service/commit/languages.go
index c23d8b8fb..8618f3f46 100644
--- a/internal/service/commit/languages.go
+++ b/internal/service/commit/languages.go
@@ -8,6 +8,7 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/linguist"
 	"gitlab.com/gitlab-org/gitaly/internal/service/ref"
@@ -77,7 +78,7 @@ func (ls languageSorter) Swap(i, j int)      { ls[i], ls[j] = ls[j], ls[i] }
 func (ls languageSorter) Less(i, j int) bool { return ls[i].Share > ls[j].Share }
 
 func lookupRevision(ctx context.Context, repoPath string, revision string) (string, error) {
-	revParse, err := helper.GitCommandReader(ctx, "--git-dir", repoPath, "rev-parse", revision)
+	revParse, err := command.Git(ctx, "--git-dir", repoPath, "rev-parse", revision)
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/service/commit/list_files.go b/internal/service/commit/list_files.go
index 083c9ba54..b994f12e2 100644
--- a/internal/service/commit/list_files.go
+++ b/internal/service/commit/list_files.go
@@ -9,6 +9,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
 )
@@ -34,7 +35,7 @@ func (s *server) ListFiles(in *pb.ListFilesRequest, stream pb.CommitService_List
 		return stream.Send(&pb.ListFilesResponse{})
 	}
 
-	cmd, err := helper.GitCommandReader(stream.Context(), "--git-dir", repoPath, "ls-tree", "-z", "-r", "--full-tree", "--full-name", "--", string(revision))
+	cmd, err := command.Git(stream.Context(), "--git-dir", repoPath, "ls-tree", "-z", "-r", "--full-tree", "--full-name", "--", string(revision))
 	if err != nil {
 		return grpc.Errorf(codes.Internal, err.Error())
 	}
diff --git a/internal/service/commit/raw_blame.go b/internal/service/commit/raw_blame.go
index 846ae1d8f..a0342e5bf 100644
--- a/internal/service/commit/raw_blame.go
+++ b/internal/service/commit/raw_blame.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 
@@ -28,7 +29,7 @@ func (s *server) RawBlame(in *pb.RawBlameRequest, stream pb.CommitService_RawBla
 	revision := string(in.GetRevision())
 	path := string(in.GetPath())
 
-	cmd, err := helper.GitCommandReader(ctx, "--git-dir", repoPath, "blame", "-p", revision, "--", path)
+	cmd, err := command.Git(ctx, "--git-dir", repoPath, "blame", "-p", revision, "--", path)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "RawBlame: cmd: %v", err)
 	}
diff --git a/internal/service/commit/testhelper_test.go b/internal/service/commit/testhelper_test.go
index 8630b8239..840636bdb 100644
--- a/internal/service/commit/testhelper_test.go
+++ b/internal/service/commit/testhelper_test.go
@@ -30,6 +30,8 @@ func TestMain(m *testing.M) {
 }
 
 func testMain(m *testing.M) int {
+	defer testhelper.MustHaveNoChildProcess()
+
 	testhelper.ConfigureRuby()
 	if err := linguist.LoadColors(); err != nil {
 		log.Fatal(err)
diff --git a/internal/service/diff/commit.go b/internal/service/diff/commit.go
index 9dd35929d..974e528cd 100644
--- a/internal/service/diff/commit.go
+++ b/internal/service/diff/commit.go
@@ -7,6 +7,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/diff"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"google.golang.org/grpc"
@@ -214,7 +215,7 @@ func validateRequest(in requestWithLeftRightCommitIds) error {
 }
 
 func eachDiff(ctx context.Context, rpc string, cmdArgs []string, limits diff.Limits, callback func(*diff.Diff) error) error {
-	cmd, err := helper.GitCommandReader(ctx, cmdArgs...)
+	cmd, err := command.Git(ctx, cmdArgs...)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "%s: cmd: %v", rpc, err)
 	}
diff --git a/internal/service/diff/testhelper_test.go b/internal/service/diff/testhelper_test.go
index b3a79c457..1c2bbb901 100644
--- a/internal/service/diff/testhelper_test.go
+++ b/internal/service/diff/testhelper_test.go
@@ -22,6 +22,12 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	os.Exit(testMain(m))
+}
+
+func testMain(m *testing.M) int {
+	defer testhelper.MustHaveNoChildProcess()
+
 	testRepo = testhelper.TestRepository()
 
 	testhelper.ConfigureRuby()
@@ -29,12 +35,9 @@ func TestMain(m *testing.M) {
 	if err != nil {
 		log.Fatal(err)
 	}
+	defer ruby.Stop()
 
-	os.Exit(func() int {
-		defer ruby.Stop()
-
-		return m.Run()
-	}())
+	return m.Run()
 }
 
 func runDiffServer(t *testing.T) *grpc.Server {
diff --git a/internal/service/ref/refexists.go b/internal/service/ref/refexists.go
index fba2f3858..6f88522cb 100644
--- a/internal/service/ref/refexists.go
+++ b/internal/service/ref/refexists.go
@@ -11,6 +11,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"golang.org/x/net/context"
 )
@@ -40,8 +41,8 @@ func refExists(ctx context.Context, repoPath string, ref string) (bool, error) {
 		return false, grpc.Errorf(codes.InvalidArgument, "invalid refname")
 	}
 
-	osCommand := exec.Command(helper.GitPath(), "--git-dir", repoPath, "show-ref", "--verify", "--quiet", ref)
-	cmd, err := helper.NewCommand(ctx, osCommand, nil, ioutil.Discard, nil)
+	osCommand := exec.Command(command.GitPath(), "--git-dir", repoPath, "show-ref", "--verify", "--quiet", ref)
+	cmd, err := command.New(ctx, osCommand, nil, ioutil.Discard, nil)
 	if err != nil {
 		return false, grpc.Errorf(codes.Internal, err.Error())
 	}
@@ -53,7 +54,7 @@ func refExists(ctx context.Context, repoPath string, ref string) (bool, error) {
 		return true, nil
 	}
 
-	if code, ok := helper.ExitStatus(err); ok && code == 1 {
+	if code, ok := command.ExitStatus(err); ok && code == 1 {
 		// Exit code 1: the ref does not exist
 		return false, nil
 	}
diff --git a/internal/service/ref/refname.go b/internal/service/ref/refname.go
index c2d1116b0..1b16a34c8 100644
--- a/internal/service/ref/refname.go
+++ b/internal/service/ref/refname.go
@@ -10,6 +10,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"golang.org/x/net/context"
 )
@@ -41,7 +42,7 @@ func findRefName(ctx context.Context, path, commitID, prefix string) (string, er
 		"prefix":    prefix,
 	}).Debug("findRefName")
 
-	cmd, err := helper.GitCommandReader(ctx, "--git-dir", path, "for-each-ref", "--format=%(refname)", "--count=1", prefix, "--contains", commitID)
+	cmd, err := command.Git(ctx, "--git-dir", path, "for-each-ref", "--format=%(refname)", "--count=1", prefix, "--contains", commitID)
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go
index 3ac10f292..38b88f799 100644
--- a/internal/service/ref/refs.go
+++ b/internal/service/ref/refs.go
@@ -12,6 +12,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
 	"golang.org/x/net/context"
@@ -51,7 +52,7 @@ func findRefs(ctx context.Context, writer lines.Sender, repo *pb.Repository, pat
 	}
 
 	args = append(args, patterns...)
-	cmd, err := helper.GitCommandReader(ctx, args...)
+	cmd, err := command.Git(ctx, args...)
 	if err != nil {
 		return err
 	}
@@ -87,7 +88,7 @@ func (s *server) FindAllTags(in *pb.FindAllTagsRequest, stream pb.RefService_Fin
 func _findBranchNames(ctx context.Context, repoPath string) ([][]byte, error) {
 	var names [][]byte
 
-	cmd, err := helper.GitCommandReader(ctx, "--git-dir", repoPath, "for-each-ref", "refs/heads", "--format=%(refname)")
+	cmd, err := command.Git(ctx, "--git-dir", repoPath, "for-each-ref", "refs/heads", "--format=%(refname)")
 	if err != nil {
 		return nil, err
 	}
@@ -111,7 +112,7 @@ func _findBranchNames(ctx context.Context, repoPath string) ([][]byte, error) {
 func _headReference(ctx context.Context, repoPath string) ([]byte, error) {
 	var headRef []byte
 
-	cmd, err := helper.GitCommandReader(ctx, "--git-dir", repoPath, "rev-parse", "--symbolic-full-name", "HEAD")
+	cmd, err := command.Git(ctx, "--git-dir", repoPath, "rev-parse", "--symbolic-full-name", "HEAD")
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/service/ref/testhelper_test.go b/internal/service/ref/testhelper_test.go
index f2de2e634..f432a64a8 100644
--- a/internal/service/ref/testhelper_test.go
+++ b/internal/service/ref/testhelper_test.go
@@ -71,9 +71,15 @@ var (
 )
 
 func TestMain(m *testing.M) {
-	var err error
+	os.Exit(testMain(m))
+}
+
+func testMain(m *testing.M) int {
+	defer testhelper.MustHaveNoChildProcess()
 
 	testRepo = testhelper.TestRepository()
+
+	var err error
 	testRepoPath, err = helper.GetRepoPath(testRepo)
 	if err != nil {
 		log.Fatal(err)
@@ -90,9 +96,7 @@ func TestMain(m *testing.M) {
 	// ref list works correctly
 	lines.MaxMsgSize = 100
 
-	os.Exit(func() int {
-		return m.Run()
-	}())
+	return m.Run()
 }
 
 func runRefServiceServer(t *testing.T) *grpc.Server {
diff --git a/internal/service/repository/fetch_remote.go b/internal/service/repository/fetch_remote.go
index ad0cb8c2d..f247b5e8d 100644
--- a/internal/service/repository/fetch_remote.go
+++ b/internal/service/repository/fetch_remote.go
@@ -12,8 +12,8 @@ import (
 	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 func (server) FetchRemote(ctx context.Context, in *pb.FetchRemoteRequest) (*pb.FetchRemoteResponse, error) {
@@ -31,7 +31,7 @@ func (server) FetchRemote(ctx context.Context, in *pb.FetchRemoteRequest) (*pb.F
 		return nil, err
 	}
 
-	cmd, err := helper.GitlabShellCommandReader(ctx, envs, "gitlab-projects", args...)
+	cmd, err := command.GitlabShell(ctx, envs, "gitlab-projects", args...)
 	if err != nil {
 		return nil, grpc.Errorf(codes.Internal, err.Error())
 	}
diff --git a/internal/service/repository/gc.go b/internal/service/repository/gc.go
index 0fc3033d9..b7c162621 100644
--- a/internal/service/repository/gc.go
+++ b/internal/service/repository/gc.go
@@ -11,6 +11,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
@@ -31,7 +32,7 @@ func (server) GarbageCollect(ctx context.Context, in *pb.GarbageCollectRequest)
 		args = append(args, "repack.writeBitmaps=false")
 	}
 	args = append(args, "gc")
-	cmd, err := helper.GitCommandReader(ctx, args...)
+	cmd, err := command.Git(ctx, args...)
 	if err != nil {
 		return nil, grpc.Errorf(codes.Internal, err.Error())
 	}
diff --git a/internal/service/repository/repack.go b/internal/service/repository/repack.go
index f54738067..55fe576ab 100644
--- a/internal/service/repository/repack.go
+++ b/internal/service/repository/repack.go
@@ -11,6 +11,7 @@ import (
 	"google.golang.org/grpc/codes"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
@@ -46,7 +47,7 @@ func repackCommand(ctx context.Context, rpcName string, repo *pb.Repository, bit
 	}
 	cmdArgs = append(cmdArgs, args...)
 
-	cmd, err := helper.GitCommandReader(ctx, cmdArgs...)
+	cmd, err := command.Git(ctx, cmdArgs...)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, err.Error())
 	}
diff --git a/internal/service/repository/size.go b/internal/service/repository/size.go
index a54440097..6d6955bd0 100644
--- a/internal/service/repository/size.go
+++ b/internal/service/repository/size.go
@@ -6,6 +6,7 @@ import (
 	"os/exec"
 	"strconv"
 
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -20,7 +21,7 @@ func (s *server) RepositorySize(ctx context.Context, in *pb.RepositorySizeReques
 		return nil, err
 	}
 
-	cmd, err := helper.NewCommand(ctx, exec.Command("du", "-sk", path), nil, nil, nil)
+	cmd, err := command.New(ctx, exec.Command("du", "-sk", path), nil, nil, nil)
 	if err != nil {
 		grpc_logrus.Extract(ctx).WithError(err).Warn("ignoring du command error")
 		return repositorySizeResponse(0), nil
diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go
index 27ee2d990..b01d3911d 100644
--- a/internal/service/smarthttp/inforefs.go
+++ b/internal/service/smarthttp/inforefs.go
@@ -8,6 +8,7 @@ import (
 	log "github.com/Sirupsen/logrus"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 
@@ -39,7 +40,7 @@ func handleInfoRefs(ctx context.Context, service string, repo *pb.Repository, w
 		return err
 	}
 
-	cmd, err := helper.GitCommandReader(ctx, service, "--stateless-rpc", "--advertise-refs", repoPath)
+	cmd, err := command.Git(ctx, service, "--stateless-rpc", "--advertise-refs", repoPath)
 	if err != nil {
 		return grpc.Errorf(codes.Internal, "GetInfoRefs: cmd: %v", err)
 	}
diff --git a/internal/service/smarthttp/receive_pack.go b/internal/service/smarthttp/receive_pack.go
index d71a3287b..e0d4fc021 100644
--- a/internal/service/smarthttp/receive_pack.go
+++ b/internal/service/smarthttp/receive_pack.go
@@ -6,6 +6,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -48,8 +49,8 @@ func (s *server) PostReceivePack(stream pb.SmartHTTPService_PostReceivePackServe
 		return err
 	}
 
-	osCommand := exec.Command(helper.GitPath(), "receive-pack", "--stateless-rpc", repoPath)
-	cmd, err := helper.NewCommand(stream.Context(), osCommand, stdin, stdout, nil, env...)
+	osCommand := exec.Command(command.GitPath(), "receive-pack", "--stateless-rpc", repoPath)
+	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, nil, env...)
 
 	if err != nil {
 		return grpc.Errorf(codes.Unavailable, "PostReceivePack: cmd: %v", err)
diff --git a/internal/service/smarthttp/upload_pack.go b/internal/service/smarthttp/upload_pack.go
index e12d82096..8baf6d851 100644
--- a/internal/service/smarthttp/upload_pack.go
+++ b/internal/service/smarthttp/upload_pack.go
@@ -5,6 +5,7 @@ import (
 	"os/exec"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -59,8 +60,8 @@ func (s *server) PostUploadPack(stream pb.SmartHTTPService_PostUploadPackServer)
 		return err
 	}
 
-	osCommand := exec.Command(helper.GitPath(), "upload-pack", "--stateless-rpc", repoPath)
-	cmd, err := helper.NewCommand(stream.Context(), osCommand, stdin, stdout, nil)
+	osCommand := exec.Command(command.GitPath(), "upload-pack", "--stateless-rpc", repoPath)
+	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, nil)
 
 	if err != nil {
 		return grpc.Errorf(codes.Unavailable, "PostUploadPack: cmd: %v", err)
@@ -69,7 +70,7 @@ func (s *server) PostUploadPack(stream pb.SmartHTTPService_PostUploadPackServer)
 
 	if err := cmd.Wait(); err != nil {
 		pw.Close() // ensure scanDeepen returns
-		if _, ok := helper.ExitStatus(err); ok && <-deepenCh {
+		if _, ok := command.ExitStatus(err); ok && <-deepenCh {
 			// We have seen a 'deepen' message in the request. It is expected that
 			// git-upload-pack has a non-zero exit status: don't treat this as an
 			// error.
diff --git a/internal/service/ssh/receive_pack.go b/internal/service/ssh/receive_pack.go
index 4fbcd490e..0fa80d6b7 100644
--- a/internal/service/ssh/receive_pack.go
+++ b/internal/service/ssh/receive_pack.go
@@ -6,6 +6,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -52,8 +53,8 @@ func (s *server) SSHReceivePack(stream pb.SSHService_SSHReceivePackServer) error
 		return err
 	}
 
-	osCommand := exec.Command(helper.GitPath(), "receive-pack", repoPath)
-	cmd, err := helper.NewCommand(stream.Context(), osCommand, stdin, stdout, stderr, env...)
+	osCommand := exec.Command(command.GitPath(), "receive-pack", repoPath)
+	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, stderr, env...)
 
 	if err != nil {
 		return grpc.Errorf(codes.Unavailable, "SSHReceivePack: cmd: %v", err)
@@ -61,7 +62,7 @@ func (s *server) SSHReceivePack(stream pb.SSHService_SSHReceivePackServer) error
 	defer cmd.Close()
 
 	if err := cmd.Wait(); err != nil {
-		if status, ok := helper.ExitStatus(err); ok {
+		if status, ok := command.ExitStatus(err); ok {
 			return helper.DecorateError(
 				codes.Internal,
 				stream.Send(&pb.SSHReceivePackResponse{ExitStatus: &pb.ExitStatus{Value: int32(status)}}),
diff --git a/internal/service/ssh/testhelper_test.go b/internal/service/ssh/testhelper_test.go
index 51ddb5895..c2d7db619 100644
--- a/internal/service/ssh/testhelper_test.go
+++ b/internal/service/ssh/testhelper_test.go
@@ -32,6 +32,12 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	os.Exit(testMain(m))
+}
+
+func testMain(m *testing.M) int {
+	defer testhelper.MustHaveNoChildProcess()
+
 	cwd = mustGetCwd()
 
 	err := os.RemoveAll(testPath)
@@ -52,9 +58,7 @@ func TestMain(m *testing.M) {
 	defer os.Remove("gitaly-receive-pack")
 	receivePackPath = path.Join(cwd, "gitaly-receive-pack")
 
-	os.Exit(func() int {
-		return m.Run()
-	}())
+	return m.Run()
 }
 
 func mustGetCwd() string {
diff --git a/internal/service/ssh/upload_pack.go b/internal/service/ssh/upload_pack.go
index fddd2ba93..811d4eb45 100644
--- a/internal/service/ssh/upload_pack.go
+++ b/internal/service/ssh/upload_pack.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc"
@@ -45,9 +46,9 @@ func (s *server) SSHUploadPack(stream pb.SSHService_SSHUploadPackServer) error {
 
 	args = append(args, "upload-pack", repoPath)
 
-	osCommand := exec.Command(helper.GitPath(), args...)
+	osCommand := exec.Command(command.GitPath(), args...)
 
-	cmd, err := helper.NewCommand(stream.Context(), osCommand, stdin, stdout, stderr)
+	cmd, err := command.New(stream.Context(), osCommand, stdin, stdout, stderr)
 
 	if err != nil {
 		return grpc.Errorf(codes.Unavailable, "SSHUploadPack: cmd: %v", err)
@@ -55,7 +56,7 @@ func (s *server) SSHUploadPack(stream pb.SSHService_SSHUploadPackServer) error {
 	defer cmd.Close()
 
 	if err := cmd.Wait(); err != nil {
-		if status, ok := helper.ExitStatus(err); ok {
+		if status, ok := command.ExitStatus(err); ok {
 			return helper.DecorateError(
 				codes.Internal,
 				stream.Send(&pb.SSHUploadPackResponse{ExitStatus: &pb.ExitStatus{Value: int32(status)}}),
diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go
index 89e4b5013..f43feb7e3 100644
--- a/internal/supervisor/supervisor.go
+++ b/internal/supervisor/supervisor.go
@@ -50,6 +50,7 @@ type Process struct {
 	dir  string
 
 	// Shutdown
+	shutdown chan struct{}
 	done     chan struct{}
 	stopOnce sync.Once
 }
@@ -61,11 +62,12 @@ func New(name string, env []string, args []string, dir string) (*Process, error)
 	}
 
 	p := &Process{
-		Name: name,
-		env:  env,
-		args: args,
-		dir:  dir,
-		done: make(chan struct{}),
+		Name:     name,
+		env:      env,
+		args:     args,
+		dir:      dir,
+		shutdown: make(chan struct{}),
+		done:     make(chan struct{}),
 	}
 
 	go watch(p)
@@ -122,10 +124,12 @@ func watch(p *Process) {
 			case <-waitCh:
 				crashes++
 				break waitLoop
-			case <-p.done:
+			case <-p.shutdown:
 				if cmd.Process != nil {
 					cmd.Process.Kill()
 				}
+				<-waitCh
+				close(p.done)
 				return
 			}
 		}
@@ -171,6 +175,12 @@ func (p *Process) Stop() {
 	}
 
 	p.stopOnce.Do(func() {
-		close(p.done)
+		close(p.shutdown)
 	})
+
+	select {
+	case <-p.done:
+	case <-time.After(1 * time.Second):
+		// Don't wait for shutdown forever
+	}
 }
diff --git a/internal/supervisor/supervisor_test.go b/internal/supervisor/supervisor_test.go
index a285b4808..8ad6d82db 100644
--- a/internal/supervisor/supervisor_test.go
+++ b/internal/supervisor/supervisor_test.go
@@ -14,6 +14,8 @@ import (
 	"testing"
 	"time"
 
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+
 	"github.com/stretchr/testify/require"
 )
 
@@ -28,8 +30,9 @@ func TestMain(m *testing.M) {
 }
 
 func testMain(m *testing.M) int {
-	var err error
+	defer testhelper.MustHaveNoChildProcess()
 
+	var err error
 	testDir, err = ioutil.TempDir("", "gitaly-supervisor-test")
 	if err != nil {
 		log.Fatal(err)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 3712b4aa9..69e47ea15 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -2,6 +2,7 @@ package testhelper
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -10,15 +11,17 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"syscall"
 	"testing"
 
 	log "github.com/Sirupsen/logrus"
 
-	"github.com/grpc-ecosystem/go-grpc-middleware"
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 
+	"github.com/grpc-ecosystem/go-grpc-middleware"
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 )
@@ -191,3 +194,45 @@ func NewTestGrpcServer(t *testing.T, streamInterceptors []grpc.StreamServerInter
 		grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer(unaryInterceptors...)),
 	)
 }
+
+// MustHaveNoChildProcess panics if it finds a running or finished child
+// process.
+func MustHaveNoChildProcess() {
+	mustFindNoFinishedChildProcess()
+	mustFindNoRunningChildProcess()
+}
+
+func mustFindNoFinishedChildProcess() {
+	// Wait4(pid int, wstatus *WaitStatus, options int, rusage *Rusage) (wpid int, err error)
+	//
+	// We use pid -1 to wait for any child. We don't care about wstatus or
+	// rusage. Use WNOHANG to return immediately if there is no child waiting
+	// to be reaped.
+	wpid, err := syscall.Wait4(-1, nil, syscall.WNOHANG, nil)
+	if err != nil {
+		return
+	}
+
+	if wpid > 0 {
+		panic(fmt.Errorf("wait4 found child process %d", wpid))
+	}
+}
+
+func mustFindNoRunningChildProcess() {
+	pgrep := exec.Command("pgrep", "-P", fmt.Sprintf("%d", os.Getpid()))
+	desc := fmt.Sprintf("%q", strings.Join(pgrep.Args, " "))
+
+	out, err := pgrep.Output()
+	if err == nil {
+		pidsComma := strings.Replace(strings.TrimSpace(string(out)), ",", "\n", -1)
+		psOut, _ := exec.Command("ps", "-o", "pid,args", "-p", pidsComma).Output()
+		panic(fmt.Sprintf("found running child processes %s:\n%s", pidsComma, psOut))
+	}
+
+	if status, ok := command.ExitStatus(err); ok && status == 1 {
+		// Exit status 1 means no processes were found
+		return
+	}
+
+	panic(fmt.Sprintf("%s: %v", desc, err))
+}