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

GUACAMOLE-600: Correct handling of non-blocking socket for timeout. #542

Merged

Conversation

necouchman
Copy link
Contributor

Okay, I think I've got the issue figured out after some Google Research on how to handle non-blocking sockets in Linux. Let me know if this looks sane or needs any work. I re-tested and am able to establish SSH connections...

@necouchman
Copy link
Contributor Author

Tested with a couple of different timeout values (5s and 30s) against an IP address that isn't up/doesn't respond, and it appears to work as expected.

@mike-jumper
Copy link
Contributor

Should we restore the default (blocking) behavior prior to returning the socket, so that callers are not surprised to find that the socket is non-blocking?

@neandrake
Copy link
Contributor

I was looking into something similar, flipping a socket to non-blocking and back, for when read-only connections are created and sync display so it doesn’t cause the main connection to lag then. I found it challenging to think through the behavior of the socket. Would the socket need polled until all expected data comes in before switching back to blocking?

@necouchman
Copy link
Contributor Author

Should we restore the default (blocking) behavior prior to returning the socket, so that callers are not surprised to find that the socket is non-blocking?

Yeah, probably a good idea. I've added a block in to do that, too.

@mike-jumper
Copy link
Contributor

Would the socket need polled until all expected data comes in before switching back to blocking?

I think the blocking vs. non-blocking behavior comes into play only when actually servicing a system call, like a read(). The socket should buffer any received data regardless of whether O_NONBLOCK is set.

@mike-jumper mike-jumper merged commit ac6f03d into apache:staging/1.6.0 Aug 30, 2024
1 check passed
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