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.

Sign up for GitHub

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

Jump to bottom

TcpReceiveSendGetsCanceledByDispose: update test for change in Linux kernel. #93198

Merged
merged 1 commit into from
Oct 13, 2023
Merged

TcpReceiveSendGetsCanceledByDispose: update test for change in Linux kernel. #93198

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive/SendReceive.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1031,11 +1031,12 @@ public async Task TcpReceiveSendGetsCanceledByDispose(bool receiveOrSend, bool i
return;
}

// RHEL7 kernel has a bug preventing close(AF_UNKNOWN) to succeed with IPv6 sockets.
// In this case Dispose will trigger a graceful shutdown, which means that receive will succeed on socket2.
// This bug is fixed in kernel 3.10.0-1160.25+.
// TODO: Remove this, once CI machines are updated to a newer kernel.
bool mayShutdownGraceful = UsesSync && PlatformDetection.IsRedHatFamily7 && receiveOrSend && (ipv6Server || dualModeClient);
// .NET uses connect(AF_UNSPEC) to abort on-going operations on Linux.
// Linux 6.4+ introduced a change (4faeee0cf8a5d88d63cdbc3bab124fb0e6aed08c) which disallows
// this operation while operations are on-going.
Comment on lines +1034 to +1036
Copy link
Member

@antonfirsov antonfirsov Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Assuming that distros will eventually update to 6.4+, does this mean that we should stop using connect(AF_UNSPEC)? Is there any alternative?

Copy link
Member Author

@tmds tmds Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think for the time being we should keep connect(AF_UNSPEC) as there are many kernels still that provide this, and the resulting behavior matches closer to what Windows does.

Copy link
Member

Choose a reason for hiding this comment

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

Eventually, though, this means our Linux implementation will not behave the way we want it to, right? Is there something better we can / should be doing instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're doing the best we can.

I've been investigating the issue with a kernel dev. He wrote this patch which fixes the regression.
We'll see if it gets included. 🤞

// When the connect fails, .NET falls back to use shutdown(SHUT_RDWR).
// This causes the receive on socket2 to succeed instead of failing with ConnectionReset.
bool mayShutdownGraceful = UsesSync && PlatformDetection.IsLinux && receiveOrSend;
antonfirsov marked this conversation as resolved.
Show resolved Hide resolved

// We try this a couple of times to deal with a timing race: if the Dispose happens
// before the operation is started, the peer won't see a ConnectionReset SocketException and we won't
Expand Down
Loading