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

Socket timeout #555

Merged
merged 3 commits into from May 24, 2023
Merged

Socket timeout #555

merged 3 commits into from May 24, 2023

Conversation

kazu-yamamoto
Copy link
Collaborator

This implements SO_SNDTIMEO and SO_RCVTIMEO.
This patch is small, so you can understand my intention very easily, I guess.
Please skip the style-only commit and review the last two commits.

--
-- Currently, the 'recv' family is blocked on Windows because a proper
-- IO manager is not implemented. To use with 'System.Timeout.timeout'
-- on Windows, use 'Network.Socket.setSocketOption' with
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a remark. We had problems with using System.Timeout.timeout on Windows, exactly because the lack of proper IO manager. On Windows the implementation is inefficient (relays on GHC scheduler, needs to create threads), and thus it can be too slow for network applications.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI: @Mistuke is preparing a new IO manager for Windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know, in the mean time (since 4 years 😁) we use (for other reasons) https://github.com/input-output-hk/Win32-network

Copy link
Collaborator

Choose a reason for hiding this comment

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

Network should be easy to convert, that is, do a 1-1 conversion. Despite the name FD network has always used native Socket handles, but the big issue is that it's of the wrong time, so it gets truncated. This is "safe" as long as you don't use enough sockets to hit the truncation mark. So the socket handle type in Network needs to be a struct so we can use FDs (int) on Unix but long long (pointers) on Windows.

After that the conversion should be very straight forward. Unfortunately my day job leaves me with a lot less time to work on Haskell in my free time. If you'd be willing to do the type conversion @coot we can move this along a bit.

@kazu-yamamoto
Copy link
Collaborator Author

Relating to #543.

@kazu-yamamoto
Copy link
Collaborator Author

@Mistuke Do you agree with this PR itself?
I need this PR for my DNS code.

@Mistuke
Copy link
Collaborator

Mistuke commented May 24, 2023

@kazu-yamamoto yes sorry, was double checking the ranges of the types.

Yeah this looks good to me.

Copy link
Collaborator

@Mistuke Mistuke left a comment

Choose a reason for hiding this comment

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

Thanks!

@kazu-yamamoto
Copy link
Collaborator Author

@Mistuke Thank you!

@kazu-yamamoto kazu-yamamoto merged commit e73616c into haskell:master May 24, 2023
17 checks passed
@kazu-yamamoto kazu-yamamoto deleted the timo branch May 24, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants