Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Test integration tests #1250
Test integration tests #1250
Changes from 18 commits
7c6af7a
4f37634
a504e27
2b20162
547e4a6
f1e652f
f2fe3e1
393a7a1
e26f86b
4cd89e3
0b96523
e7611a5
91e2ce5
83e9b20
2a891a6
ceeb17c
1d3b07e
11aac88
401600d
8b9cae2
179fa7f
c088277
fe010a4
7a4e418
89e84c1
9246e91
05f4a28
2b30996
aeb1492
d768f19
fabf8c7
1b6c73b
49c730c
5129976
06a52e3
a0d6360
914f419
5426cf1
63dbf1c
82cf3bb
cca8ba6
63ef6ef
b2b0b88
6d4e59f
093193c
624486c
df3fb94
24e76e4
ecc0fae
309ae1b
5bcc756
eee3db5
70ab38d
b9764c2
9944991
30a3620
19b0ac6
e10fc14
cf26535
4aebfd2
9399fd9
46cbe42
850e87c
0a0a473
0eb40d6
48ce561
ec33d6d
65e84f9
f850d9b
68cc3a8
03df17d
4336148
2210fa1
64bb26b
e732710
c7c0b66
25e0aac
0c1d4af
ca4536e
e0f9dd0
d96c7f7
edce619
b378cf1
980dbf5
81f8d03
f3ca592
cf23073
06a7444
52d9c68
c2143d3
769863f
e398e63
0251c81
502ef8a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did these changes fix something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the new Dotnet, I wanted to try the new overloaded method
ConnectAsync
https://learn.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.connectasync?view=net-7.0#system-net-sockets-socket-connectasync(system-net-endpoint-system-threading-cancellationtoken)and
ReceiveAsync
https://learn.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.receiveasync?view=net-7.0#system-net-sockets-socket-receiveasync(system-memory((system-byte))-system-net-sockets-socketflags-system-threading-cancellationtoken)What do you think, should I revert this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to revert the
Connect
changes, for 2 reasons:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I am fine with removing the
#elif FEATURE_SOCKET_APM
and#elif FEATURE_SOCKET_TAP
blocks)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will revert some changes. I did some of them because I wanted to try if something helped.
SocketExtensions
because every target framework has aFEATURE_SOCKET_EAP
flag.FEATURE_SOCKET_TAP
andFEATURE_SOCKET_APM
have not been used. I think that for the modern .NET we can propose a better sync solution. I'm open to suggestions.AsyncHelper
from https://github.com/aspnet/AspNetIdentity/blob/main/src/Microsoft.AspNet.Identity.Core/AsyncHelper.cs I think that this code works well. But no matter, I'll probably revert it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FEATURE_SOCKET_EAP
), andConnectCore
doesn't useSocketExtensions
AsyncHelper
is doing) and then wait for it on the main thread anyway. (I don't think it will block a threadpool thread as I claimed earlier, so it's not a huge problem)