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.

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

net: TestListenerClose failures with "Dial to closed TCP listener succeeded" #38700

Closed
bcmills opened this issue Apr 27, 2020 · 7 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Apr 27, 2020

2020-04-27T15:50:54-70d9b72/linux-386-longtest

--- FAIL: TestListenerClose (0.00s)
    --- FAIL: TestListenerClose/tcp (0.00s)
        net_test.go:277: Dial to closed TCP listener succeeded.
FAIL
FAIL	net	29.631s

See previously #14124.

CC @mikioh @bradfitz @ianlancetaylor

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 27, 2020
@bcmills bcmills added this to the Backlog milestone Apr 27, 2020
@bcmills bcmills changed the title net: TestListenerClose failure on linux-386-longtest net: TestListenerClose failures with "Dial to closed TCP listener succeeded" Sep 2, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Nov 11, 2021

greplogs --dashboard -md -l -e 'FAIL: TestListenerClose' --since=2021-09-03

2021-11-11T04:54:05-4b27d40/dragonfly-amd64
2021-11-04T02:38:03-747e4af/linux-amd64-longtest
2021-11-02T20:47:30-b246873/linux-386-longtest

@bcmills
Copy link
Contributor Author

bcmills commented Dec 9, 2021

greplogs --dashboard -md -l -e 'FAIL: TestListenerClose.*\n(?: .*\n)*.*Dial to closed TCP listener succeeded' --since=2021-11-12

2021-12-09T19:01:08-8ff254e/linux-amd64-staticlockranking

@bcmills
Copy link
Contributor Author

bcmills commented Dec 9, 2021

This test is failing with some regularity on builders for first-class ports (notably linux-386 and linux-amd64). To my mind, that makes it a release-blocker via #11811.

Either we should fix the net package to satisfy the expectations of the test, or we should fix the test to match the realities of the net package. (If nothing else, the failing check could be removed from the test without rendering it useless.)

@bcmills bcmills added okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker labels Dec 9, 2021
@bcmills bcmills modified the milestones: Backlog, Go1.18 Dec 9, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Dec 9, 2021

Notably, this assertion begins with a time.Sleep for an arbitrary duration, and then asserts that a Dial call does not succeed:

go/src/net/net_test.go

Lines 273 to 279 in f229e70

time.Sleep(time.Millisecond)
cc, err := Dial("tcp", dst)
if err == nil {
t.Error("Dial to closed TCP listener succeeded.")
cc.Close()
}

If I understand correctly, the test does not (and cannot feasibly) ensure that the port is not simultaneously reused by some other test — or even some entirely different process! — on the system, since the TCP TIME_WAIT state only strictly applies to the four-tuple of (source address, source port, destination address, destination port). net.Dial specifies only the destination address and port, so a new listener may immediately be opened on the same address and port as long as it only accepts connections from new source addresses and/or ports. Thus, the Dial may spuriously succeed by connecting to an entirely new listener at the same port.

@bcmills bcmills self-assigned this Dec 9, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Dec 9, 2021

I'll send a CL to remove the erroneous assertion.

@gopherbot
Copy link

Change https://golang.org/cl/370666 mentions this issue: net: remove erroneous Dial check in TestListenerClose

@rsc rsc unassigned bcmills Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants