Skip to content

Move Select_AcceptNonBlocking_Success to non-parallel collection#57503

Merged
antonfirsov merged 1 commit into
dotnet:mainfrom
antonfirsov:sockets/Select_AcceptNonBlocking_Success
Aug 17, 2021
Merged

Move Select_AcceptNonBlocking_Success to non-parallel collection#57503
antonfirsov merged 1 commit into
dotnet:mainfrom
antonfirsov:sockets/Select_AcceptNonBlocking_Success

Conversation

@antonfirsov

@antonfirsov antonfirsov commented Aug 16, 2021

Copy link
Copy Markdown
Contributor

On Unix, this test seems to be prone to IPv4-IPv6 port collision. Windows failures can be likely explained by timing issues on CI (5 seconds can be strict with heavy parallel workloads). Serialization should help with both problems.

Fixes #41579

@ghost

ghost commented Aug 16, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

On Unix, this test seems to be prone to IPv4-IPv6 port collision. Windows failures can be likely explained by timing issues on the CI (5 seconds can be strict with heavy parallel workloads), serialization should help with that as well.

Fixes #41579

Author: antonfirsov
Assignees: -
Labels:

area-System.Net.Sockets

Milestone: -

@antonfirsov

Copy link
Copy Markdown
Contributor Author

/azp run runtime-libraries-coreclr outerloop

@antonfirsov
antonfirsov requested a review from a team August 16, 2021 18:52
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@antonfirsov

Copy link
Copy Markdown
Contributor Author

The only outerloop test failure is #1722.

@antonfirsov
antonfirsov merged commit 9b93ac6 into dotnet:main Aug 17, 2021
@karelz karelz added this to the 6.0.0 milestone Aug 17, 2021
@tmds

tmds commented Aug 17, 2021

Copy link
Copy Markdown
Member

This is the test:

        [OuterLoop]
        [Fact]
        public static async Task Select_AcceptNonBlocking_Success()
        {
            using (Socket listenSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
            {
                int port = listenSocket.BindToAnonymousPort(IPAddress.Loopback);

                listenSocket.Blocking = false;

                listenSocket.Listen(5);

                Task t = Task.Run(() => { DoAccept(listenSocket, 5); });

                // Loop, doing connections and pausing between
                for (int i = 0; i < 5; i++)
                {
                    Thread.Sleep(50);
                    using (Socket connectSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
                    {
                        connectSocket.Connect(listenSocket.LocalEndPoint);
                    }
                }

                // Give the task 5 seconds to complete; if not, assume it's hung.
                await t.WaitAsync(TimeSpan.FromSeconds(5));
            }
        }

On Unix, this test seems to be prone to IPv4-IPv6 port collision.

@antonfirsov what does this mean? How does it fail?

Windows failures can be likely explained by timing issues on CI (5 seconds can be strict with heavy parallel workloads).

This about the Task.Run not all having run within the 5 second timeout?
Should we not increase the timeout?

@antonfirsov

antonfirsov commented Aug 17, 2021

Copy link
Copy Markdown
Contributor Author

On Unix, this test seems to be prone to IPv4-IPv6 port collision.
@antonfirsov what does this mean? How does it fail?

I realized that I was likely wrong with this. Even if DoAccept accepts some unrelated sockets, it shouldn't timeout.

Should we not increase the timeout?

In my experience this is insufficient on a slow overloaded CI machine, we don't really know how much the timeout should be. Disabling parallelism usually makes thing much more deterministic.

@tmds do you think we may be masking a bug with the serialization?

@tmds

tmds commented Aug 18, 2021

Copy link
Copy Markdown
Member

@tmds do you think we may be masking a bug with the serialization?

No.

I guess solving IPv4-IPv6 port collision was the main motivation for serializing this test.

The drawback of serialization is that it increases the test run duration.
Timeouts of 30 seconds or even 60 seconds are not uncommon in the tests.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failure: System.Net.Sockets.Tests.SelectTest.Select_AcceptNonBlocking_Success

4 participants