1
0
mirror of https://github.com/golang/go synced 2024-11-26 23:21:22 -07:00

net/http/fcgi: remove locking added to prevent a test-only race

The race reported in issue #41167 was detected only because the
ReadWriter used in test code happened to be a bytes.Buffer whose
Read and Write operate (unsafely) on shared state. This is not the
case in any realistic scenario where the FastCGI protocol is spoken
over sockets or pairs of pipes.

Since tests that use nopWriteCloser don't care about any output
generate by child.Serve(), we change nopWriteCloser to provide
a dummy Write method.

Remove the locking added in CL 252417, since it causes a deadlock
during write as reported in #43901. The race in tests no longer
happens thanks to the aforementioned change to nopWriteCloser.

Fixes #43901.
Updates #41167.

Change-Id: I8cf31088a71253c34056698f8e2ad0bee9fcf6c6
GitHub-Last-Rev: b06d8377fd
GitHub-Pull-Request: golang/go#43027
Reviewed-on: https://go-review.googlesource.com/c/go/+/275692
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
This commit is contained in:
Hilko Bengen 2021-01-25 22:54:20 +00:00 committed by Ian Lance Taylor
parent bf0f7c9d78
commit ce8b318624
2 changed files with 8 additions and 7 deletions

View File

@ -171,12 +171,9 @@ func (c *child) serve() {
defer c.cleanUp()
var rec record
for {
c.conn.mutex.Lock()
if err := rec.read(c.conn.rwc); err != nil {
c.conn.mutex.Unlock()
return
}
c.conn.mutex.Unlock()
if err := c.handleRecord(&rec); err != nil {
return
}

View File

@ -221,7 +221,11 @@ var cleanUpTests = []struct {
}
type nopWriteCloser struct {
io.ReadWriter
io.Reader
}
func (nopWriteCloser) Write(buf []byte) (int, error) {
return len(buf), nil
}
func (nopWriteCloser) Close() error {
@ -235,7 +239,7 @@ func TestChildServeCleansUp(t *testing.T) {
for _, tt := range cleanUpTests {
input := make([]byte, len(tt.input))
copy(input, tt.input)
rc := nopWriteCloser{bytes.NewBuffer(input)}
rc := nopWriteCloser{bytes.NewReader(input)}
done := make(chan bool)
c := newChild(rc, http.HandlerFunc(func(
w http.ResponseWriter,
@ -325,7 +329,7 @@ func TestChildServeReadsEnvVars(t *testing.T) {
for _, tt := range envVarTests {
input := make([]byte, len(tt.input))
copy(input, tt.input)
rc := nopWriteCloser{bytes.NewBuffer(input)}
rc := nopWriteCloser{bytes.NewReader(input)}
done := make(chan bool)
c := newChild(rc, http.HandlerFunc(func(
w http.ResponseWriter,
@ -375,7 +379,7 @@ func TestResponseWriterSniffsContentType(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
input := make([]byte, len(streamFullRequestStdin))
copy(input, streamFullRequestStdin)
rc := nopWriteCloser{bytes.NewBuffer(input)}
rc := nopWriteCloser{bytes.NewReader(input)}
done := make(chan bool)
var resp *response
c := newChild(rc, http.HandlerFunc(func(