Extend SendAsync/ReceiveAsync cancellation tests #44161
Conversation
|
Tagging subscribers to this area: @dotnet/ncl |
| [Theory] | ||
| [InlineData(false)] | ||
| [InlineData(true)] | ||
| public async Task SendAsync_CanceledDuringOperation_Throws(bool ipv6) |
There was a problem hiding this comment.
This test seems highly timing-dependent.
I think what you are doing here is. you're flooding the kernel send buffer with so much data (64K per send, 1000 sends) that you're assuming at least some of the send tasks won't complete before you call CancelAfter. Am I understanding this correctly?
I don't see any better way to do this really, but I worry we may see intermittent failures here. Thoughts?
At the very least this should be [Outerloop] I think.
There was a problem hiding this comment.
Also a comment explaining what's intended would be useful.
There was a problem hiding this comment.
Actually, since this is TCP (I was thinking UDP for some reason) I think what you're doing should be fine.
That said, it should be [Outerloop] and a comment would be nice.
There was a problem hiding this comment.
you're flooding the kernel send buffer with so much data (64K per send, 1000 sends) that you're assuming at least some of the send tasks won't complete before you call CancelAfter
Yes correct. I also tried to change the send buffer size in the hope that this will fragment and slow down the execution of the sends, but on windows this didn't change anything. Maybe I have a bad understanding on how send buffer works.
but I worry we may see intermittent failures here
I think this can only happen if an environment is so fast, that all the sends complete before the cancellation. I havent seen this yet with the current parameters. Or do you rather worry because of the side effects this may have on other tests running in parallel?
| const int NumOfSends = 1000; | ||
|
|
||
| (Socket client, Socket server) = SocketTestExtensions.CreateConnectedSocketPair(ipv6); | ||
| byte[] buffer = new byte[8192 * 8]; |
There was a problem hiding this comment.
Nit: it's easier to read buffer size if you do it as "64 * 1024"
|
@geoffkizer can you have another look? I don't think the test is timing-critical in it's current form (see #44161 (comment)). What is the |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Test failure is #44214. |
ghost
locked as resolved and limited conversation to collaborators
CanceledDuringOperation_ThrowstoReceiveAsync_CanceledDuringOperation_Throwsand extend it to IPv6 socketsSocket.SendAsync. Unless I'm missing something, this method was missing functional coverage.@geoffkizer @stephentoub @tmds PTAL. I want to find the best practices for testing cancellation for stuff in #43935.