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

Update TIdIOHandler.ConnectTimeout logic #3

Open
rlebeau opened this issue Apr 5, 2017 · 5 comments
Open

Update TIdIOHandler.ConnectTimeout logic #3

rlebeau opened this issue Apr 5, 2017 · 5 comments
Assignees
Labels
Element: I/O Handlers Issues related to TIdIOHandler and descendants Element: Socket Stacks Issues related to OS socket APIs, TIdStack and TIdSocketList descedants, etc Status: In Progress Issue is being worked on Type: Enhancement Issue is proposing a new feature/enhancement

Comments

@rlebeau
Copy link
Member

rlebeau commented Apr 5, 2017

Update TIdIOHandler to pass its ConnectTimeout value all the way to TIdStack.Connect() to get the timeout logic out of TIdIOHandler.ConnectClient(). This way, TIdStackWindows can be updated to use WSA...() functions when available (ie, WSAConnectByName() or WSAConnectByList(), which both have a timeout parameter), and thus avoid having to use the TIdConnectThread class to abort the connection after the timeout elapsed. Other TIdStack... classes can continue using TIdConnectThread when suitable platform-specific techniques are not available.

@rlebeau rlebeau added Type: Enhancement Issue is proposing a new feature/enhancement Element: I/O Handlers Issues related to TIdIOHandler and descendants labels Apr 5, 2017
@rlebeau rlebeau added this to the Indy 12 milestone Apr 5, 2017
@rlebeau
Copy link
Member Author

rlebeau commented Jun 12, 2020

Alternatively, update TIdIOHandlerStack.ConnectClient() to put the socket into non-blocking mode, if possible, before calling connect(), then use select() to handle the timeout, and then restore the socket to blocking mode if the connection is successful.

@rlebeau
Copy link
Member Author

rlebeau commented Apr 26, 2023

From duplicate #466

I have an application where I need a very short connect timeout.
Then I just saw that Indy starts a new thread for this connect ...!?
I took the time to implement a version based on non blocking connect and select... see attached patch.
The idea is to split TIdSocketHandle.Connect into GStack.Connect (which starts an non-blocking connect) and later UpdateBinding.
The code in TIdIOHandlerStack.ConnectClient just starts connect then waits with select (write) and then decides to UpdateBinding or raise exception.
I also added the TIdSocketListWindows.SelectWrite... why was it not yet included??
It's just a prof of concept code, no AntiFreeze and no posix variant.

indy_asyc_connect.patch

@gec75
Copy link

gec75 commented Apr 26, 2023

I don't really care about small implementation details :), the idea is that matters.
But is there a reason why Select(write) aka. "Writable" is NOT exposed somehow?

@rlebeau rlebeau self-assigned this Apr 26, 2023
@rlebeau
Copy link
Member Author

rlebeau commented Apr 26, 2023

is there a reason why Select(write) aka. "Writable" is NOT exposed somehow?

There is currently nothing in Indy that waits for a socket to be in a writable state before writing data to it, Indy just writes and lets it block as needed. Indy is designed around blocking I/O, not non-blocking I/O. So, there just hasn't been any need for a high-level wrapper to call select() for writing, only for reading (since there are a few places where Indy does wait for readable states).

That being said, Indy does expose one way to call select() for writing - the TIdSocketList class has a public virtual Select() class method which has an AWriteList parameter which is fully implemented, just never used by Indy itself. But I don't see why you could not call it directly if you needed to.

@rlebeau
Copy link
Member Author

rlebeau commented Jul 19, 2023

Apparently, a recent change in Linux now prevents one thread from disconnecting a TCP socket if other threads are waiting on the socket!

This does not bode well for Indy, which expects to be able to shutdown/close a TCP socket while threads are blocked on socket I/O operations. For this ticket, this change means that using connect() in non-blocking mode now becomes more important, otherwise ConnectTimeout becomes useless as a blocked connect() will no longer unblock when shutdown() is called.

Will have to review other socket functions for potential problems introduced by this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Element: I/O Handlers Issues related to TIdIOHandler and descendants Element: Socket Stacks Issues related to OS socket APIs, TIdStack and TIdSocketList descedants, etc Status: In Progress Issue is being worked on Type: Enhancement Issue is proposing a new feature/enhancement
Projects
None yet
Development

No branches or pull requests

2 participants