Fix ConnectAsync not throwing exception for already connected Socket#27173
Fix ConnectAsync not throwing exception for already connected Socket#27173wtgodbe merged 1 commit intodotnet:masterfrom wtgodbe:SocketAsync
Conversation
wtgodbe
added
the
* NO MERGE *
|
@dotnet-bot test outerloop_netcoreapp_ubuntu16.10_release |
|
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug |
| } | ||
| } | ||
|
|
||
| [OuterLoop] // TODO: Issue #11345 |
|
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug |
1 similar comment
|
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug |
|
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug |
wtgodbe
changed the title
wtgodbe
removed
the
* NO MERGE *
|
@dotnet-bot help |
|
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
The following jobs are launched by default for each PR against dotnet/corefx:master. Click to expand
The following optional jobs are available in PRs against dotnet/corefx:master. Click to expand
Have a nice day! |
|
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
The following jobs are launched by default for each PR against dotnet/corefx:master. Click to expand
The following optional jobs are available in PRs against dotnet/corefx:master. Click to expand
Have a nice day! |
|
@dotnet-bot test Outerloop Windows x64 Debug Build |
| { | ||
| // Throw up front if we are already connected | ||
| if (this.Connected) | ||
| { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Good call. Do you think we need to add the check to the other versions of BeginConnect, such as
? I only added it to the code paths we hit in the test cases so far.There was a problem hiding this comment.
Yes, we should be consistent about doing the check in all variations of Connect/BeginConnect.
geoffkizer
left a comment
geoffkizer
left a comment
There was a problem hiding this comment.
Looks fine in general, one comment above.
|
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug |
|
@geoffkizer checks have been updated, and the WebSockets test that was failing in the last commit is passing for me locally. |
|
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 |
|
LGTM |
|
Looks like test failures are on tests added 2 hours ago - 5b51318. I'll try rebasing. |
|
@dotnet-bot test outerloop netcoreapp Ubuntu16.10 Debug |
|
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. |
|
The netfx failure is #27363. I have a fix out. You can ignore. |
|
@geoffkizer in that case, pending Jeremy's confirmation about the Crypto failures, is this good to merge? |
|
Yep, go for it |
|
@wtgodbe If it was Decrypt_512_CekDoesNotDecrypt_FixedValue, that was my bug, and is fixed now. |
Commit migrated from dotnet/corefx@fb17ad8
6 participants
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.