fix(race-conditon): fix data race condition when accessing stderr on pipe

[mahadzaryab1]

There was a race condition that existed in Pipe.Exec, which spins up a goroutine that access Pipe.stderr without a lock. This PR fixes that by protecting Pipe.stderr with Pipe.mu. This PR fixes #193.

Testing
Created a simple unit test as follows:

func TestDataRaceStdErr(t *testing.T) {
	stdOut := new(bytes.Buffer)
	stdErr := new(bytes.Buffer)

	_, err := script.Exec("echo").WithStdout(stdOut).WithStderr(stdErr).Stdout()
	if err != nil {
		t.Fatal(err)
	}
}

Running go test -race before the changes in this PR outputted the same warning seen in the issue. Running it again with the changes in the PR cause the tests to pass.

[mahadzaryab1]

question: we currently do not run go test in the CI with the -race flag so I removed the regression test. Do we want to run the tests with the -race flag? If so, should I add the regression test back in?

[bitfield]

If we can have race conditions, we should definitely run CI tests with the race detector on!

[mahadzaryab1]

If we can have race conditions, we should definitely run CI tests with the race detector on!

Added the -race flag to ci.yml and added a regression test.

[bitfield]

Looking good so far!

[bitfield]

@mahadzaryab1 I'm happy if you're happy—do you want to go ahead and do a final quality pass, checking documentation comments, alphabetical order, gotestdox-friendly test names, and so on? You know the house style now 😄

[mahadzaryab1]

@mahadzaryab1 I'm happy if you're happy—do you want to go ahead and do a final quality pass, checking documentation comments, alphabetical order, gotestdox-friendly test names, and so on? You know the house style now 😄

@bitfield Done! Let me know if there's anything I missed.

[bitfield]

Super nice, @mahadzaryab1! Thanks a lot. I'll wait till you're done on #208, and then we can roll all these changes up into a new release.