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

Disabling two socket tests #93916

Merged
merged 3 commits into from
Oct 29, 2023
Merged

Conversation

liveans
Copy link
Member

@liveans liveans commented Oct 24, 2023

Disables #93737 and #93735 until regression in Linux kernel has been fixed.
Tracking issue: #94149

/cc @tmds @wfurt @antonfirsov

@ghost
Copy link

ghost commented Oct 24, 2023

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

Issue Details

Disables #93737 and #93735 until the regression has been fixed.

/cc @tmds @wfurt @antonfirsov

Author: liveans
Assignees: liveans
Labels:

area-System.Net.Sockets

Milestone: -

@liveans liveans requested a review from a team October 24, 2023 11:11
@liveans
Copy link
Member Author

liveans commented Oct 24, 2023

/azp list

@azure-pipelines
Copy link

CI/CD Pipelines for this repository:

@liveans
Copy link
Member Author

liveans commented Oct 24, 2023

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@karelz karelz added this to the 9.0.0 milestone Oct 24, 2023
Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these tests are only failing for sync variants.

If we don't think we'll get them reenabled quickly, it may be worth scoping the condition to just those scenarios to avoid losing too much coverage. E.g.

if (UsesSync && !PlatformDetection.IsWindows)
{
    // [ActiveIssue("https://github.com/dotnet/runtime/issues/93737", TestPlatforms.Linux)]
    return;
}

@karelz
Copy link
Member

karelz commented Oct 24, 2023

Is it worth merging the 2 bugs and keep just one to track the Linux kernel regression?
(and disable both tests against the one bug)

@liveans
Copy link
Member Author

liveans commented Oct 25, 2023

@tmds is there any tracking issue for the regression that happened recently on the Linux kernel?

@tmds
Copy link
Member

tmds commented Oct 25, 2023

@tmds is there any tracking issue for the regression that happened recently on the Linux kernel?

There are a few issues for failing tests due to the regression. No specific issue is used to track the regression afaik.

@liveans
Copy link
Member Author

liveans commented Oct 29, 2023

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liveans liveans merged commit b6b00ec into dotnet:main Oct 29, 2023
174 checks passed
@@ -137,6 +137,12 @@ public async Task Connect_AfterDisconnect_Fails()
[InlineData("[::ffff:1.1.1.1]", true, false)]
public async Task ConnectGetsCanceledByDispose(string addressString, bool useDns, bool owning)
{
if (UsesSync && !PlatformDetection.IsWindows)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there need to disable this also on macOS? It feels like !IsWindows should really be IsLinux????
Also this is regression in particular Linux Kernels. At that seems rare at this moment. It may be worth of adding version check instead of blindly disabling Linux runs -> it should still work on most supported distributions AFAIK.

Copy link
Member Author

@liveans liveans Oct 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I've missed it. It should work as expected on macOS, thanks for catching that. I'll change it with another PR.

For the version controlling, AFAIK, only the latest kernel versions have this problem. Is there any example of version checking on Linux in .NET?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth of adding version check instead of blindly disabling Linux runs

The change is being backported to some kernels, since it's considered to be security sensitive. I think it would be very difficult if not impossible to determine only by kernel version if a system is impacted.

@liveans liveans deleted the socket-failure-disable branch October 30, 2023 10:07
liveans added a commit to liveans/dotnet-runtime that referenced this pull request Nov 9, 2023
* Disable two socket tests

* Revert close to dispose

* Disable tests against one issue
@ghost ghost locked as resolved and limited conversation to collaborators Nov 29, 2023
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.

6 participants