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

fix #196 #197

Closed
wants to merge 1 commit into from
Closed

fix #196 #197

wants to merge 1 commit into from

Conversation

antonio-quarta
Copy link

I think that this commit would fix the issue #196

As reported here:
https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-commtimeouts

A value of MAXDWORD, combined with zero values for both the ReadTotalTimeoutConstant and ReadTotalTimeoutMultiplier members, specifies that the read operation is to return immediately with the bytes that have already been received, even if no bytes have been received.

@sirhcel
Copy link
Contributor

sirhcel commented Jul 10, 2024

Thank you very much for looking into this and creating this PR @antonio-quarta! With #79, we've had a similar one around for quite a while and I've integrated this one about the time you created yours.

The changes from #79 already got released with 4.4.0. Does this solve the issue for you as well? Or is there a special reason for leaving ReadTotalTimeoutMultiplier at 0 instead of setting it to MAXDWORD as mentioned in the citation in the description?

@RossSmyth
Copy link
Contributor

Yeah this has already been fixed in the latest release, so I would say this PR should probably be closed.

@sirhcel
Copy link
Contributor

sirhcel commented Jul 31, 2024

The changes from #79 already got released with 4.4.0. Does this solve the issue for you as well? Or is there a special reason for leaving ReadTotalTimeoutMultiplier at 0 instead of setting it to MAXDWORD as mentioned in the citation in the description?

I read the citation wrong in the first place. What it says is what we are doing with #79 and I will close #196.

@sirhcel sirhcel closed this Jul 31, 2024
@antonio-quarta
Copy link
Author

Sorry for late response, thank you for fixing this! 👍

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.

3 participants