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: b06d8377fdada075775d79a20577d38a7c471b45
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>

diff --git a/src/net/http/fcgi/child.go b/src/net/http/fcgi/child.go
index e97b8440e1d372a71782eb8ada4261a7b83f0cfc..756722ba1412a2dcc4e55a2f6e1e6ea4772d3ef6 100644 (file)
--- a/src/net/http/fcgi/child.go
+++ b/src/net/http/fcgi/child.go
@@ -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
                }

diff --git a/src/net/http/fcgi/fcgi_test.go b/src/net/http/fcgi/fcgi_test.go
index d3b704f8219b907057742f99dbf92cd98c9a09da..b58111de208dee2e05f8f46096bddba1f1a7ec74 100644 (file)
--- a/src/net/http/fcgi/fcgi_test.go
+++ b/src/net/http/fcgi/fcgi_test.go
@@ -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(