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

Socket async cancellation and timeout during connection #2860

Closed
roji opened this issue Feb 21, 2020 · 6 comments · Fixed by #3167
Closed

Socket async cancellation and timeout during connection #2860

roji opened this issue Feb 21, 2020 · 6 comments · Fixed by #3167
Assignees
@roji
Labels
bug
Milestone
5.0.0

Comments

@roji
Copy link
Member

roji commented Feb 21, 2020

The connector's Connect method doesn't set the socket timeout, so if there's a network failure right during connection we may end up blocking forever.

Take this opportunity to also look at the async path, and to make sure the connection timeout is enforced there as well.
@roji roji added the bug label Feb 21, 2020
@roji roji added this to the 5.0 milestone Feb 21, 2020
@roji roji self-assigned this Feb 21, 2020
@roji roji mentioned this issue Feb 21, 2020
Closed
@joelnotified joelnotified mentioned this issue Mar 6, 2020
Closed
@roji
Copy link
Member Author

roji commented Sep 17, 2020

@vonzshik another timeout-related issue just in case you're interested :)

@vonzshik
Copy link
Contributor

@roji while I don't mind taking this one, it might take a little more effort than just setting the timeout for the connection, as we're also missing it for the authentication part. But I'll do my best to get it done for the 5.0 release.

@roji
Copy link
Member Author

roji commented Sep 18, 2020

No problem @vonzshik! I'll clear the assignee, if you see you're not getting around to it for 5.0, I can try to work on it too. Let's see later.

@roji roji removed their assignment Sep 18, 2020
@roji
Copy link
Member Author

roji commented Sep 20, 2020

Note: thanks to dotnet/runtime#40750, we can do both cancellation and timeout (via cancellation) in Socket.ConnectAsync.

@vonzshik
Copy link
Contributor

Woohoo!

@roji roji self-assigned this Sep 20, 2020
@roji roji mentioned this issue Sep 20, 2020
Closed
16 tasks
@roji
Copy link
Member Author

roji commented Sep 20, 2020

@vonzshik I'm adding support for this, a PR will be up shortly. I'm going to scope this issue to track specifically ConnectAsync, and have opened #3166 to track any remaining cancellation-related tasks (which you're welcome to take a look at).

@roji roji mentioned this issue Sep 20, 2020
Merged
roji added a commit to roji/Npgsql that referenced this issue Sep 20, 2020
@roji
Real cancellation and timeout for ConnectAsync
62d1f7c 
Also enables the code analysis rule for flowing cancellation tokens
(part of npgsql#3162).

Fixes npgsql#2860
roji added a commit that referenced this issue Sep 20, 2020
@roji
Real cancellation and timeout for ConnectAsync (#3167)
83e8d90 
Also enables the code analysis rule for flowing cancellation tokens
(part of #3162).

Fixes #2860
@roji roji changed the title Socket timeout not set during connection Socket async cancellation and timeout during connection Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@roji roji

Labels
bug
Projects
None yet
Milestone
5.0.0
Development

Successfully merging a pull request may close this issue.

Real cancellation and timeout for ConnectAsync roji/Npgsql
2 participants
@roji @vonzshik