net/http/fcgi: race detected during execution of TestResponseWriterSniffsContentType test #41167
Comments
dmitshur
added
the
NeedsInvestigation
|
Probably a connection lock is needed for this read, https://github.com/golang/go/blob/master/src/net/http/fcgi/child.go#L158 writes from |
|
Change https://golang.org/cl/252417 mentions this issue: |
|
Hi, I have looked into this a bit more and I tried to backport the racy test to 1.15. The race was already there and was not introduced by the security release. The only difference is that now we cover that codepath with tests, so I think this race has been there for a long time. |
|
Change https://golang.org/cl/252717 mentions this issue: |
|
Sounds good. Since the race isn't new and critical, the fix can be made available in Go 1.16 and we don't need to backport it. However, let's skip the new test so that the race builders continue to pass and provide full coverage on the release branches. @gopherbot Please backport to both Go 1.15 and 1.14, but only the test fix. |
|
Backport issue(s) opened: #41192 (for 1.14), #41193 (for 1.15), #41194 (for 1.16). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
|
Change https://golang.org/cl/252718 mentions this issue: |
dmitshur
added
NeedsFix
A test introduced in the security release is flaky due to a pre-existing issue that does not qualify for backport itself. Updates #41167 Fixes #41193 Change-Id: Ie6014e0796c1baee7b077881b5a799f9947fc9c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/252717 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
A test introduced in the security release is flaky due to a pre-existing issue that does not qualify for backport itself. Updates #41167 Fixes #41192 Change-Id: Ie6014e0796c1baee7b077881b5a799f9947fc9c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/252718 Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
A test introduced in the security release is flaky due to a pre-existing issue that does not qualify for backport itself. Updates golang#41167 Fixes golang#41193 Change-Id: Ie6014e0796c1baee7b077881b5a799f9947fc9c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/252717 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
|
Change https://golang.org/cl/275692 mentions this issue: |
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: b06d837 GitHub-Pull-Request: #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>
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
I looked at https://build.golang.org/?branch=release-branch.go1.15 (and https://build.golang.org/?branch=release-branch.go1.14) and spotted a few new failures on race builders. Then I reproduced it locally by running:
What did you expect to see?
What did you see instead?
/cc @FiloSottile @empijei
The text was updated successfully, but these errors were encountered: