net: TestAcceptTimeout still flaky #17948
Comments
|
CL https://golang.org/cl/33330 mentions this issue. |
|
CL https://golang.org/cl/33258 mentions this issue. |
|
Nope. Still flaky: freebsd-amd64-gce101 at 81627f0 https://build.golang.org/log/c69b9de990f663a7ea8e00a0f2b589365234aaba |
It's too flaky and doing more harm than good. Disable it until it can be made reliable. Updates #17948 Updates #17927 Change-Id: Iaab7f09a4060da377fcd3ca2262527fef50c558d Reviewed-on: https://go-review.googlesource.com/33330 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
That's a different issue. We fixed two issues: a) context-based canceling based on defer function execution behaves differently than previous versions on Darwin and FreeBSD, b) TCP control segments for active close operation at passive open side don't deliver immediately on Darwin, and now have c) "dial tcp 127.0.0.1:42120: i/o timeout" at active open side. It's fine to move forward step by step, I'm not sure why the test starts behaving unbridledly during Go 1.8 development cycle, though. Does Go 1.8 contain massive changes to runtime, net or testing packages? |
In any case, it's still flaky.
There have certainly been runtime changes. You would know whether there have been net changes. I don't think there have been notable testing changes. |
bradfitz
added
the
Testing
quentinmit
added
the
NeedsFix
|
Change https://golang.org/cl/105095 mentions this issue: |
|
This test has been sitting in the tree with an unconditional skip for a long time. If it is actually useful we should fix it properly; otherwise we should get rid of it. |
|
Change https://go.dev/cl/548940 mentions this issue: |
|
Change https://go.dev/cl/557438 mentions this issue: |
…tation This ensures that if the listener has already timed out when Accept is called, Accept always returns an error instead of instantaneously accepting a connection that was already pending. For #17948. Change-Id: Iabef7121590df3dcc2fe428429d7c2bc2bcb6cd5 Reviewed-on: https://go-review.googlesource.com/c/go/+/557438 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>
|
Still flaky, but now with |
|
Change https://go.dev/cl/558175 mentions this issue: |
Also use DialContext instead of just Dial so that we can ensure the call returns before we close the listener. The Dial in this test is intended to complete before the call to Accept, but there is no synchronization to ensure that and sometimes it doesn't happen. That's ok and mostly immaterial to the test, but it does mean we need to ignore Dial errors (which can happen when the listener is closed), and we need to be a little more careful about not dialing a port that may have already been reused by some other test. Fixes #65240. Updates #17948. Change-Id: Ife1b5c3062939441b58f4c096461bf5d7841889b Reviewed-on: https://go-review.googlesource.com/c/go/+/558175 Reviewed-by: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
…tation This ensures that if the listener has already timed out when Accept is called, Accept always returns an error instead of instantaneously accepting a connection that was already pending. For golang#17948. Change-Id: Iabef7121590df3dcc2fe428429d7c2bc2bcb6cd5 Reviewed-on: https://go-review.googlesource.com/c/go/+/557438 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>
This test has been unconditionally skipped for over five years. It may be that whatever was causing it to flake has been fixed. And if it hasn't been fixed, it isn't providing any value. Let's unskip it for the Go 1.23 development cycle and see what happens. Let's also use a separate listener for each test case, so that a leaked Dial goroutine from one case won't interfere with the other. Fixes golang#17948 (maybe). Change-Id: I239f22ca5d5a44388b9aa0ed4d81e451c6342617 Reviewed-on: https://go-review.googlesource.com/c/go/+/548940 Commit-Queue: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Also use DialContext instead of just Dial so that we can ensure the call returns before we close the listener. The Dial in this test is intended to complete before the call to Accept, but there is no synchronization to ensure that and sometimes it doesn't happen. That's ok and mostly immaterial to the test, but it does mean we need to ignore Dial errors (which can happen when the listener is closed), and we need to be a little more careful about not dialing a port that may have already been reused by some other test. Fixes golang#65240. Updates golang#17948. Change-Id: Ife1b5c3062939441b58f4c096461bf5d7841889b Reviewed-on: https://go-review.googlesource.com/c/go/+/558175 Reviewed-by: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TestAcceptTimeoutis still flaky:https://build.golang.org/log/74ef15095b884eff2e47c1d1ed91a7643c2424cb
I'm just going to disable it for now. It's doing more harm than good.
The text was updated successfully, but these errors were encountered: