Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net/http/cgi: TestCopyError failures due to unexpected child process #57369

Closed
gopherbot opened this issue Dec 17, 2022 · 9 comments
Closed

net/http/cgi: TestCopyError failures due to unexpected child process #57369

gopherbot opened this issue Dec 17, 2022 · 9 comments
Assignees
@bcmills
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.22

Comments

@gopherbot
Copy link
Contributor

gopherbot commented Dec 17, 2022 •
edited by bcmills
Loading 

#!watchflakes
post <- goos == "darwin" && pkg == "net/http/cgi" && test == "TestCopyError" && `expected child to be gone`

Issue created automatically to collect these failures.

Example (log):

2022/12/08 14:53:02 httptest.Server blocked in Close after 5 seconds, waiting for connections:
  *net.TCPConn 0xc000228030 127.0.0.1:50046 in state active
2022/12/08 14:53:03 cgi: copy error: write tcp 127.0.0.1:50045->127.0.0.1:50046: write: broken pipe
--- FAIL: TestCopyError (21.17s)
    host_test.go:425: post-conn.Close, expected child to be gone
2022/12/08 14:53:03 cgi: copy error: past write limit
2022/12/08 14:53:03 cgi: no headers
2022/12/08 14:53:03 cgi: missing required Content-Type in headers
2022/12/08 14:53:03 cgi: no headers

— watchflakes
@gopherbot gopherbot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 17, 2022
@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "net/http/cgi" && test == "TestCopyError"
2022-12-08 18:29 darwin-amd64-12_0 go@f368abb4 net/http/cgi.TestCopyError (log) 
2022/12/08 14:53:02 httptest.Server blocked in Close after 5 seconds, waiting for connections:
  *net.TCPConn 0xc000228030 127.0.0.1:50046 in state active
2022/12/08 14:53:03 cgi: copy error: write tcp 127.0.0.1:50045->127.0.0.1:50046: write: broken pipe
--- FAIL: TestCopyError (21.17s)
    host_test.go:425: post-conn.Close, expected child to be gone
2022/12/08 14:53:03 cgi: copy error: past write limit
2022/12/08 14:53:03 cgi: no headers
2022/12/08 14:53:03 cgi: missing required Content-Type in headers
2022/12/08 14:53:03 cgi: no headers

— watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "net/http/cgi" && test == "TestCopyError"
2023-02-15 21:52 darwin-amd64-12_0 go@7b398b1f net/http/cgi.TestCopyError (log) 
2023/02/16 00:25:31 cgi: copy error: write tcp 127.0.0.1:50013->127.0.0.1:50014: write: connection reset by peer
--- FAIL: TestCopyError (19.33s)
    host_test.go:425: post-conn.Close, expected child to be gone
2023/02/16 00:25:31 cgi: copy error: past write limit
2023/02/16 00:25:32 cgi: no headers
2023/02/16 00:25:32 cgi: missing required Content-Type in headers
2023/02/16 00:25:32 cgi: no headers

— watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "net/http/cgi" && test == "TestCopyError"
2023-04-27 22:26 darwin-amd64-13 go@afe2d222 net/http/cgi.TestCopyError (log) 
2023/04/27 18:40:28 httptest.Server blocked in Close after 5 seconds, waiting for connections:
  *net.TCPConn 0xc00003e068 127.0.0.1:50048 in state active
2023/04/27 18:40:37 cgi: copy error: write tcp 127.0.0.1:50047->127.0.0.1:50048: write: broken pipe
--- FAIL: TestCopyError (29.13s)
    host_test.go:425: post-conn.Close, expected child to be gone
2023/04/27 18:40:38 cgi: copy error: past write limit
2023/04/27 18:40:38 cgi: no headers
2023/04/27 18:40:39 cgi: missing required Content-Type in headers
2023/04/27 18:40:39 cgi: no headers

— watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "net/http/cgi" && test == "TestCopyError"
2023-05-11 19:07 darwin-amd64-13 go@a2737b1a net/http/cgi.TestCopyError (log) 
2023/05/12 04:06:15 cgi: copy error: write tcp 127.0.0.1:50045->127.0.0.1:50046: write: broken pipe
--- FAIL: TestCopyError (28.73s)
    host_test.go:425: post-conn.Close, expected child to be gone
2023/05/12 04:06:16 cgi: copy error: past write limit
2023/05/12 04:06:16 cgi: no headers
2023/05/12 04:06:16 cgi: missing required Content-Type in headers
2023/05/12 04:06:16 cgi: no headers

— watchflakes

@gopherbot

This comment was marked as off-topic.

Sign in to view
@gopherbot

This comment was marked as off-topic.

Sign in to view
@bcmills bcmills mentioned this issue Nov 3, 2023
Open
@bcmills
Copy link
Contributor

bcmills commented Nov 3, 2023

This looks like another instance of #63937.

@bcmills bcmills added this to the Backlog milestone Nov 3, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/554075 mentions this issue: net/http/cgi: in TestCopyError, check for a Handler.ServeHTTP goroutine instead of a running PID

@bcmills
Copy link
Contributor

bcmills commented Jan 4, 2024

This looks like another instance of #63937.

Hmm, maybe not. The test can also also fail spuriously if the CGI process's PID is quickly reused by another long-running process (such as another test running in parallel).

@bcmills bcmills self-assigned this Jan 4, 2024
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. labels Jan 4, 2024
@bcmills bcmills modified the milestones: Backlog, Go1.22 Jan 4, 2024
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 4, 2024
@bcmills bcmills changed the title net/http/cgi: TestCopyError failures net/http/cgi: TestCopyError failures due to unexpected child process Jan 4, 2024
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
@ezz-no
net/http/cgi: in TestCopyError, check for a Handler.ServeHTTP gorouti…
5c54a90 
…ne instead of a running PID

Previously, the test could fail spuriously if the CGI process's PID
happened to be reused in between checks. That sort of reuse is highly
unlikely on platforms that cycle through the PID space sequentially
(such as Linux), but plausible on platforms that use randomized PIDs
(such as OpenBSD).

Also unskip the test on Windows, since it no longer relies on being
able to send signal 0 to an arbitrary PID.

Also change the expected failure mode of the test to a timeout instead
of a call to t.Fatalf, so that on failure we get a useful goroutine
dump for debugging instead of a non-actionable failure message.

Fixes golang#57369 (maybe).

Change-Id: Ib7e3fff556450b48cb5e6ea120fdf4d53547479b
Reviewed-on: https://go-review.googlesource.com/c/go/+/554075
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
@golang golang locked and limited conversation to collaborators Jan 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bcmills bcmills

Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Test Flakes
Archived in project
Milestone
Go1.22
Development

No branches or pull requests
2 participants
@bcmills @gopherbot