Fix ConnectAsync not throwing exception for already connected Socket #27173

wtgodbe · 2018-02-15T21:10:11Z

For issue https://github.com/dotnet/corefx/issues/22765. @geoffkizer @wfurt PTAL

The previously failing tests are now passing for me locally, running a full test suite to make sure this doesn't regress anything else.

wtgodbe · 2018-02-15T21:13:10Z

@dotnet-bot test outerloop_netcoreapp_ubuntu16.10_release

wtgodbe · 2018-02-15T21:31:46Z

@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Release

wtgodbe · 2018-02-21T20:55:38Z

src/System.Net.Sockets/tests/FunctionalTests/Connect.cs

@@ -44,9 +44,7 @@ public abstract class Connect<T> : SocketTestHelperBase<T> where T : SocketHelpe
            }
        }

-        [OuterLoop] // TODO: Issue #11345


TODO - put this back in

wtgodbe · 2018-02-21T21:12:35Z

@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Release

wtgodbe · 2018-02-21T21:37:34Z

@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Release

wtgodbe · 2018-02-21T21:47:15Z

@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug

wtgodbe · 2018-02-21T22:16:10Z

@dotnet-bot help

dotnet-bot · 2018-02-21T22:16:16Z

Welcome to the dotnet/corefx Repository

The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR

The following commands are valid for all PRs and repositories.

Click to expand

Comment Phrase	Action
@dotnet-bot test this please	Re-run all legs. Use sparingly
@dotnet-bot test ci please	Generates (but does not run) jobs based on changes to the groovy job definitions in this branch
@dotnet-bot help	Print this help message

The following jobs are launched by default for each PR against dotnet/corefx:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test Alpine.3.6 x64 Debug Build	Alpine.3.6 x64 Debug Build
@dotnet-bot test Linux x64 Release Build	Linux x64 Release Build
@dotnet-bot test OSX x64 Debug Build	OSX x64 Debug Build
@dotnet-bot test Packaging All Configurations x64 Debug Build	Packaging All Configurations x64 Debug Build
@dotnet-bot test Windows x64 Debug Build	Windows x64 Debug Build
@dotnet-bot test Windows x86 Release Build	Windows x86 Release Build
@dotnet-bot test NETFX x86 Release Build	NETFX x86 Release Build

The following optional jobs are available in PRs against dotnet/corefx:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test Outerloop Alpine.3.6 x64 Debug Build	Queues Outerloop Alpine.3.6 x64 Debug Build
@dotnet-bot test Alpine.3.6 x64 Release Build	Queues Alpine.3.6 x64 Release Build
@dotnet-bot test Outerloop Alpine.3.6 x64 Release Build	Queues Outerloop Alpine.3.6 x64 Release Build
@dotnet-bot test CentOS.6 x64 Debug Build	Queues CentOS.6 x64 Debug Build
@dotnet-bot test Outerloop CentOS.6 x64 Debug Build	Queues Outerloop CentOS.6 x64 Debug Build
@dotnet-bot test CentOS.6 x64 Release Build	Queues CentOS.6 x64 Release Build
@dotnet-bot test Outerloop CentOS.6 x64 Release Build	Queues Outerloop CentOS.6 x64 Release Build
@dotnet-bot test Linux x64 Debug Build	Queues Linux x64 Debug Build
@dotnet-bot test Outerloop Linux x64 Debug Build	Queues Outerloop Linux x64 Debug Build
@dotnet-bot test Outerloop Linux x64 Release Build	Queues Outerloop Linux x64 Release Build
@dotnet-bot test Outerloop OSX x64 Debug Build	Queues Outerloop OSX x64 Debug Build
@dotnet-bot test OSX x64 Release Build	Queues OSX x64 Release Build
@dotnet-bot test Outerloop OSX x64 Release Build	Queues Outerloop OSX x64 Release Build
@dotnet-bot test Outerloop Packaging All Configurations x64 Debug Build	Queues Outerloop Packaging All Configurations x64 Debug Build
@dotnet-bot test Packaging All Configurations x86 Debug Build	Queues Packaging All Configurations x86 Debug Build
@dotnet-bot test Outerloop Packaging All Configurations x86 Debug Build	Queues Outerloop Packaging All Configurations x86 Debug Build
@dotnet-bot test Packaging All Configurations x64 Release Build	Queues Packaging All Configurations x64 Release Build
@dotnet-bot test Outerloop Packaging All Configurations x64 Release Build	Queues Outerloop Packaging All Configurations x64 Release Build
@dotnet-bot test Packaging All Configurations x86 Release Build	Queues Packaging All Configurations x86 Release Build
@dotnet-bot test Outerloop Packaging All Configurations x86 Release Build	Queues Outerloop Packaging All Configurations x86 Release Build
@dotnet-bot test Outerloop Windows x64 Debug Build	Queues Outerloop Windows x64 Debug Build
@dotnet-bot test Windows x86 Debug Build	Queues Windows x86 Debug Build
@dotnet-bot test Outerloop Windows x86 Debug Build	Queues Outerloop Windows x86 Debug Build
@dotnet-bot test Windows x64 Release Build	Queues Windows x64 Release Build
@dotnet-bot test Outerloop Windows x64 Release Build	Queues Outerloop Windows x64 Release Build
@dotnet-bot test Outerloop Windows x86 Release Build	Queues Outerloop Windows x86 Release Build
@dotnet-bot test NETFX x64 Debug Build	Queues NETFX x64 Debug Build
@dotnet-bot test Outerloop NETFX x64 Debug Build	Queues Outerloop NETFX x64 Debug Build
@dotnet-bot test NETFX x86 Debug Build	Queues NETFX x86 Debug Build
@dotnet-bot test Outerloop NETFX x86 Debug Build	Queues Outerloop NETFX x86 Debug Build
@dotnet-bot test NETFX x64 Release Build	Queues NETFX x64 Release Build
@dotnet-bot test Outerloop NETFX x64 Release Build	Queues Outerloop NETFX x64 Release Build
@dotnet-bot test Outerloop NETFX x86 Release Build	Queues Outerloop NETFX x86 Release Build

Have a nice day!

dotnet-bot · 2018-02-21T22:17:16Z

Welcome to the dotnet/corefx Repository

The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR

The following commands are valid for all PRs and repositories.

Click to expand

Comment Phrase	Action
@dotnet-bot test this please	Re-run all legs. Use sparingly
@dotnet-bot test ci please	Generates (but does not run) jobs based on changes to the groovy job definitions in this branch
@dotnet-bot help	Print this help message

The following jobs are launched by default for each PR against dotnet/corefx:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test Linux arm Release Build	Linux arm Release Build
@dotnet-bot test Tizen armel Debug Build	Tizen armel Debug Build

The following optional jobs are available in PRs against dotnet/corefx:master.

Click to expand

Comment Phrase	Job Launched
@dotnet-bot test innerloop CentOS7.1 Debug	Queues Innerloop CentOS7.1 Debug x64 Build and Test
@dotnet-bot test innerloop CentOS7.1 Release	Queues Innerloop CentOS7.1 Release x64 Build and Test
@dotnet-bot test code coverage	Queues Code Coverage Windows Debug
@dotnet-bot test innerloop Debian8.4 Debug	Queues Innerloop Debian8.4 Debug x64 Build and Test
@dotnet-bot test innerloop Debian8.4 Release	Queues Innerloop Debian8.4 Release x64 Build and Test
@dotnet-bot test innerloop Fedora24 Debug	Queues Innerloop Fedora24 Debug x64 Build and Test
@dotnet-bot test innerloop Fedora24 Release	Queues Innerloop Fedora24 Release x64 Build and Test
@dotnet-bot test Linux arm Debug	Queues Linux arm Debug Build
@dotnet-bot test code formatter check	Queues Code Formatter Check
@dotnet-bot test innerloop OSX10.12 Debug	Queues Innerloop OSX10.12 Debug x64 Build and Test
@dotnet-bot test innerloop OSX10.12 Release	Queues Innerloop OSX10.12 Release x64 Build and Test
@dotnet-bot test outerloop netcoreapp CentOS7.1 Debug	Queues OuterLoop netcoreapp CentOS7.1 Debug x64
@dotnet-bot test outerloop netcoreapp CentOS7.1 Release	Queues OuterLoop netcoreapp CentOS7.1 Release x64
@dotnet-bot test outerloop netcoreapp Debian8.4 Debug	Queues OuterLoop netcoreapp Debian8.4 Debug x64
@dotnet-bot test outerloop netcoreapp Debian8.4 Release	Queues OuterLoop netcoreapp Debian8.4 Release x64
@dotnet-bot test outerloop netcoreapp Fedora24 Debug	Queues OuterLoop netcoreapp Fedora24 Debug x64
@dotnet-bot test outerloop netcoreapp Fedora24 Release	Queues OuterLoop netcoreapp Fedora24 Release x64
@dotnet-bot test outerloop netcoreapp OSX10.12 Debug	Queues OuterLoop netcoreapp OSX10.12 Debug x64
@dotnet-bot test outerloop netcoreapp OSX10.12 Release	Queues OuterLoop netcoreapp OSX10.12 Release x64
@dotnet-bot test outerloop netcoreapp PortableLinux Debug	Queues OuterLoop netcoreapp PortableLinux Debug x64
@dotnet-bot test outerloop netcoreapp PortableLinux Release	Queues OuterLoop netcoreapp PortableLinux Release x64
@dotnet-bot test outerloop netcoreapp RHEL7.2 Debug	Queues OuterLoop netcoreapp RHEL7.2 Debug x64
@dotnet-bot test outerloop netcoreapp RHEL7.2 Release	Queues OuterLoop netcoreapp RHEL7.2 Release x64
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Debug	Queues OuterLoop netcoreapp Ubuntu14.04 Debug x64
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Release	Queues OuterLoop netcoreapp Ubuntu14.04 Release x64
@dotnet-bot test outerloop netcoreapp Ubuntu16.04 Debug	Queues OuterLoop netcoreapp Ubuntu16.04 Debug x64
@dotnet-bot test outerloop netcoreapp Ubuntu16.04 Release	Queues OuterLoop netcoreapp Ubuntu16.04 Release x64
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug	Queues OuterLoop netcoreapp Ubuntu16.10 Debug x64
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Release	Queues OuterLoop netcoreapp Ubuntu16.10 Release x64
@dotnet-bot test outerloop netcoreapp Windows 7 Debug	Queues OuterLoop netcoreapp Windows 7 Debug x64
@dotnet-bot test outerloop netcoreapp Windows 7 Release	Queues OuterLoop netcoreapp Windows 7 Release x64
@dotnet-bot test outerloop netcoreapp Windows_NT Debug	Queues OuterLoop netcoreapp Windows_NT Debug x64
@dotnet-bot test outerloop netcoreapp Windows_NT Release	Queues OuterLoop netcoreapp Windows_NT Release x64
@dotnet-bot test innerloop PortableLinux Debug	Queues Innerloop PortableLinux Debug x64 Build and Test
@dotnet-bot test innerloop PortableLinux Release	Queues Innerloop PortableLinux Release x64 Build and Test
@dotnet-bot test innerloop RHEL7.2 Debug	Queues Innerloop RHEL7.2 Debug x64 Build and Test
@dotnet-bot test innerloop RHEL7.2 Release	Queues Innerloop RHEL7.2 Release x64 Build and Test
@dotnet-bot test Tizen armel Release	Queues Tizen armel Release Build
@dotnet-bot test innerloop Ubuntu14.04 Debug	Queues Innerloop Ubuntu14.04 Debug x64 Build and Test
@dotnet-bot test innerloop Ubuntu14.04 Release	Queues Innerloop Ubuntu14.04 Release x64 Build and Test
@dotnet-bot test innerloop Ubuntu16.04 Debug	Queues Innerloop Ubuntu16.04 Debug x64 Build and Test
@dotnet-bot test innerloop Ubuntu16.04 Release	Queues Innerloop Ubuntu16.04 Release x64 Build and Test
@dotnet-bot test innerloop Ubuntu16.10 Debug	Queues Innerloop Ubuntu16.10 Debug x64 Build and Test
@dotnet-bot test innerloop Ubuntu16.10 Release	Queues Innerloop Ubuntu16.10 Release x64 Build and Test
@dotnet-bot test innerloop Windows_NT Debug	Queues Innerloop Windows_NT Debug x86 Build and Test
@dotnet-bot test innerloop Windows_NT Release	Queues Innerloop Windows_NT Release x64 Build and Test

Have a nice day!

wtgodbe · 2018-02-21T22:17:30Z

@dotnet-bot test Outerloop Windows x64 Debug Build

geoffkizer · 2018-02-21T22:23:07Z

src/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs

@@ -115,6 +115,11 @@ private Task<Socket> AcceptAsyncApm(Socket acceptSocket)

        internal Task ConnectAsync(EndPoint remoteEP)
        {
+            // Throw up front if we are already connected
+            if (this.Connected)
+            {


Since this and the other methods in this file are just wrappers to BeginConnect, I don't think we need to check this here, do we?

Good call. Do you think we need to add the check to the other versions of BeginConnect, such as

corefx/src/System.Net.Sockets/src/System/Net/Sockets/Socket.cs

Line 2131 in 40cbd56

public IAsyncResult BeginConnect(string host, int port, AsyncCallback requestCallback, object state)

? I only added it to the code paths we hit in the test cases so far.

Yes, we should be consistent about doing the check in all variations of Connect/BeginConnect.

geoffkizer

Looks fine in general, one comment above.

wtgodbe · 2018-02-21T22:45:27Z

@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Release
@dotnet-bot test Outerloop Windows x64 Debug Build

wtgodbe · 2018-02-21T23:06:53Z

@geoffkizer checks have been updated, and the WebSockets test that was failing in the last commit is passing for me locally.

wtgodbe · 2018-02-21T23:32:03Z

Mission Control doesn't actually show any test failures for the failing Windows job - https://mc.dot.net/#/user/wtgodbe/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Fmaster~2F/test~2Ffunctional~2Fcli~2F/fdf2e53ca078f7a23cdb16df66ee642bb0d79ad6

Here's the link to the job: https://ci3.dot.net/job/dotnet_corefx/job/master/job/windows-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_true_prtest/259/

Not sure what's going on there, so will run again.

@dotnet-bot test Outerloop Windows x64 Debug Build

geoffkizer · 2018-02-21T23:33:02Z

LGTM

wtgodbe · 2018-02-21T23:59:42Z

Looks like test failures are on tests added 2 hours ago - 5b51318. I'll try rebasing.

wtgodbe · 2018-02-22T00:02:32Z

@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug
@dotnet-bot test outerloop netcoreapp Ubuntu14.04 Release
@dotnet-bot test Outerloop Windows x64 Debug Build

wtgodbe · 2018-02-22T19:49:58Z

@bartonjs one of the outerloop tests you added in 5b51318 is failing in this job (in the Ubuntu Outerloop jobs), does it seem related?

wtgodbe · 2018-02-22T19:51:10Z

The NetFx failure is in System.Net.Http.Functional.Tests - I'm not seeing such a failure in other recent jobs, so it must be related. I'll take a look. As for the Outerloop Windows x64 job, that job has been failing consistently for a while on all PRs, and the test results in MC show no failures, so that seems unrelated.

geoffkizer · 2018-02-22T20:08:05Z

The netfx failure is #27363. I have a fix out. You can ignore.

wtgodbe · 2018-02-22T20:21:09Z

@geoffkizer in that case, pending Jeremy's confirmation about the Crypto failures, is this good to merge?

geoffkizer · 2018-02-22T20:23:25Z

Yep, go for it

bartonjs · 2018-02-22T20:34:40Z

@wtgodbe If it was Decrypt_512_CekDoesNotDecrypt_FixedValue, that was my bug, and is fixed now.

Commit migrated from dotnet/corefx@fb17ad8

wtgodbe added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 15, 2018

davidsh added the area-System.Net.Sockets label Feb 15, 2018

wtgodbe closed this Feb 15, 2018

wtgodbe reopened this Feb 15, 2018

wtgodbe commented Feb 21, 2018

View reviewed changes

wtgodbe changed the title ~~DNM - testing socket issue~~ Fix ConnectAsync not throwing exception for already connected Socket Feb 21, 2018

wtgodbe removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 21, 2018

wtgodbe requested review from geoffkizer and wfurt February 21, 2018 21:56

geoffkizer reviewed Feb 21, 2018

View reviewed changes

geoffkizer suggested changes Feb 21, 2018

View reviewed changes

geoffkizer approved these changes Feb 21, 2018

View reviewed changes

Fix Socket Async connect issue on Linux

c23717d

wtgodbe merged commit fb17ad8 into dotnet:master Feb 22, 2018

karelz added this to the 2.1.0 milestone Mar 10, 2018

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Fix Socket Async connect issue on Linux (dotnet/corefx#27173)

692ac60

Commit migrated from dotnet/corefx@fb17ad8

stephentoub mentioned this pull request Jun 22, 2023

Relaxing IsConnected Check for Datagram Sockets dotnet/runtime#87916

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ConnectAsync not throwing exception for already connected Socket #27173

Fix ConnectAsync not throwing exception for already connected Socket #27173

wtgodbe commented Feb 15, 2018 •

edited

Loading

wtgodbe commented Feb 15, 2018

wtgodbe commented Feb 15, 2018

wtgodbe Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

dotnet-bot commented Feb 21, 2018

dotnet-bot commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

geoffkizer Feb 21, 2018

wtgodbe Feb 21, 2018

geoffkizer Feb 21, 2018

geoffkizer left a comment

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

geoffkizer commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 22, 2018

wtgodbe commented Feb 22, 2018 •

edited

Loading

wtgodbe commented Feb 22, 2018

geoffkizer commented Feb 22, 2018

wtgodbe commented Feb 22, 2018

geoffkizer commented Feb 22, 2018

bartonjs commented Feb 22, 2018

Fix ConnectAsync not throwing exception for already connected Socket #27173

Fix ConnectAsync not throwing exception for already connected Socket #27173

Conversation

wtgodbe commented Feb 15, 2018 • edited Loading

wtgodbe commented Feb 15, 2018

wtgodbe commented Feb 15, 2018

wtgodbe Feb 21, 2018

Choose a reason for hiding this comment

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

dotnet-bot commented Feb 21, 2018

dotnet-bot commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

geoffkizer Feb 21, 2018

Choose a reason for hiding this comment

wtgodbe Feb 21, 2018

Choose a reason for hiding this comment

geoffkizer Feb 21, 2018

Choose a reason for hiding this comment

geoffkizer left a comment

Choose a reason for hiding this comment

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

geoffkizer commented Feb 21, 2018

wtgodbe commented Feb 21, 2018

wtgodbe commented Feb 22, 2018

wtgodbe commented Feb 22, 2018 • edited Loading

wtgodbe commented Feb 22, 2018

geoffkizer commented Feb 22, 2018

wtgodbe commented Feb 22, 2018

geoffkizer commented Feb 22, 2018

bartonjs commented Feb 22, 2018

wtgodbe commented Feb 15, 2018 •

edited

Loading

wtgodbe commented Feb 22, 2018 •

edited

Loading