Closing a TCP/IP component connection the right way

[Ahmed Sayed]

Hi,

Let's say that, I have a TCP component with its connection opened with the server, whether it is TIdTCPClient, TIdHTTP, TIdFTP, etc. What is the correct way to close the connection without raising any exception especially if the server went down before doing so?

I know that all of these classes inherit from TIdTCPClientCustom so there must be a common way to close them on that class level.

The idea here is that I want to create something similar to std::unique_lock, std::unique_ptr, and std::lock_guard. Let's call it unique_connection<T> for instance. So, regardless of the template T which can be any class that inherits from TIdTCPClientCustom, whenever unique_connection goes out of scope it will close the connection without raising any exceptions whether the server is still running or not.

I need this to make sure that there are no resources like sockets, or connections that are still open before the client gets destroyed.

[rlebeau]

I know that all of these classes inherit from TIdTCPClientCustom so there must be a common way to close them on that class level.

Simply call their Disconnect() method, which is inherited from TIdTCPConnection. It should not raise an exception, but if it does, just catch it.

In the case of TIdFTP and various other higher-level protocol components, Disconnect() will send a goodbye message to the peer before shutting down the connection. If you don't want to send that message, Disconnect() has an optional ANotifyPeer parameter that you can set to false.

I need this to make sure that there are no resources like sockets, or connections that are still open before the client gets destroyed.

You don't need to worry about that. The client's destructor will shutdown the connection and close the socket if it is still active.

[Ahmed Sayed]

I know that all of these classes inherit from TIdTCPClientCustom so there must be a common way to close them on that class level.

Simply call their Disconnect() method, which is inherited from TIdTCPConnection.  It should not raise an exception, but if it does, just catch it.

In the case of TIdFTP and various other higher-level protocol components, Disconnect() will send a goodbye message to the peer before shutting down the connection.  If you don't want to send that message, Disconnect() has an optional ANotifyPeer parameter that you can set to false.

I need this to make sure that there are no resources like sockets, or connections that are still open before the client gets destroyed.

You don't need to worry about that.  The client's destructor will shutdown the connection and close the socket if it is still active.

The way I know it is that to close the connection from the client side, I have to do something similar to this:

Code:

IdHTTP->IOHandler->WriteBufferClear();
IdHTTP->IOHandler->InputBuffer->Clear();
IdHTTP->IOHandler->Close();
IdHTTP->DisconnectNotifyPeer();

[rlebeau]

The way I know it is that to close the connection from the client side, I have to do something similar to this:

Code:

IdHTTP->IOHandler->WriteBufferClear();
IdHTTP->IOHandler->InputBuffer->Clear();
IdHTTP->IOHandler->Close();
IdHTTP->DisconnectNotifyPeer();

Most of that stuff is unnecessary:

• You don't need to call WriteBufferClear() if you don't call WriteBufferOpen() first. Even then, you don't need to call WriteBufferClear() directly if you are using WriteBuffer(Close|Cancel|Flush)() properly to begin with. Also, IOHandler->Open() and IOHandler->Close() both call WriteBufferClear() for you.
  
  
• You don't need to Close() the IOHandler manually, let Disconnect() handle that for you.
  
  
• The only time you should ever need to call InputBuffer->Clear() directly is if you need to reconnect a client after closing a connection prematurely and left unread data in the InputBuffer. The Connect() method raises an exception if Connected() returns True, which it does if there is unread data in the InputBuffer. Under normal usages, if a client follows its protocol properly and you Disconnect() cleanly, there should not be any unread data left behind. Typically, you would Clear() the InputBuffer only after an error had occurred that made you forcibly close the socket.
  
  
• You don't need to Close() the IOHandler manually, let Disconnect() handle that for you.
  
  
• Don't call DisconnectNotifyPeer() directly, call Disconnect() instead with its ANotifyPeer parameter set to true. But in your particular example, that doesn't matter because DisconnectNotifyPeer() doesn't do anything in TIdHTTP, as there is no goodbye message to send to an HTTP server. But even if it did, calling DisconnectNotifyPeer() after Close() is just plain backwards, you can't send anything to the peer after closing the connection.
  

That being said, HTTP is not a stateful protocol, and TIdHTTP manages the socket connection internally for you, connecting and disconnecting it as needed. You should never have to disconnect TIdHTTP manually.

[Justin Case]

Can I chime in here and also ask... What happens if one side of the connection doesn't send a quit type of message but just disconnects? - or the connection is lost (eg isp failure, ethernet cable gets yanked out the wall etc) ?

Does OnDisconnect() still get triggered?

[rlebeau]

What happens if one side of the connection doesn't send a quit type of message but just disconnects?

Are you asking about the client side or the server side?

On the server side, you will still get an OnDisconnect event triggered. That is not dependent on any bye/quit message.

On the client side, nothing will happen unless you try to read/write data after the disconnect occurred, in which case you will then get an exception, such as EIdConnClosedGracefully (if the disconnect is graceful and intentional) or EIdSocketError or similar otherwise.

or the connection is lost (eg isp failure, ethernet cable gets yanked out the wall etc) ?

Nothing will happen until the OS deems the connection is actually lost and starts reporting errors on it, which can take time. Until then, the connection is just sitting idle. Remember, TCP is designed to survive outages and recover from them when possible.

So, if you try to read data on a connection that is in this state, it will simply drain the InputBuffer as normal until it can't satisfy further reads, and then it will wait on the socket until new data arrives or an error occurs.

And, if you try to write data, it will continue to be buffered inside the socket as normal until the buffer is full, at which time the socket will block further writes until the OS flushes the buffer or an error occurs.

Does OnDisconnect() still get triggered?

On the server side, yes (eventually - consider using read timeouts or TCP keep-alives to lower the time needed to detect lost connections).

On the client side, the OnDisconnect event is not asynchronous. It is simply triggered when the app explicitly Disconnect's the client while it is still in an open state.

[Ahmed Sayed]

Hi again,
I am working on a Push notification service that uses a mix of indy HTTP and TCP components. So, I use HTTP to pull any pending large number of messages to the user and TCP on both sides to push and receive anything written to the socket. Let's say that the connection dropped for any reason.

Now, on the client side, I have a timer or a worker thread looping to read anything that gets sent to the client but I am always checking if the connection is on via the "Connected" method. My question is, is that enough to know that the connection has been lost, or do I need to read from the socket to raise the proper errors accordingly? Knowing that the client app should not raise exceptions in the user's face everything should be handled internally.

[rlebeau]

I am always checking if the connection is on via the "Connected" method.

You should not be doing that. Just have your timer/thread read normally and let Indy raise an exception if the connection is closed.

My question is, is that enough to know that the connection has been lost, or do I need to read from the socket to raise the proper errors accordingly?

Since Indy uses blocking sockets, a socket read is necessary. But Indy will read from the socket for you when its InputBuffer doesn't have enough bytes to satisfy a read operation. You don't need to read from the socket yourself, such as with the Connected() method (which does a socket read internally).

Knowing that the client app should not raise exceptions in the user's face everything should be handled internally.

Simply wrap any read/write operation you perform in a try..except (Pascal) or try..catch (C++) block. Indy uses exceptions for all of its error reporting.