BUG in TIdTcpClient

This is the forum for miscellaneous technical/programming questions.

Moderator: 2ffat

BUG in TIdTcpClient

Postby theLizard » Thu Aug 11, 2016 7:17 pm

It is my opinion that there is a bug in TidTcpClient.

This bug relates to TidTcpClient->Connect() an EIdConnectTimeout Exception is NOT always triggered when there is NO physical connection to the network.

Follow the code..

Code: Select all
try
      {
      tcp->ConnectTimeout = 200;

      if(!tcp->Connected())
         tcp->Connect();  //at this point I would expect an EIdConnectTimeout  Exception

      tcp->IOHandler->InputBuffer->Clear();

      tcp->IOHandler->WriteDirect(writeA, 19, 0);  //I would also expect a timeout here

      if(tcp->IOHandler->Readable(500))            //I would NOT expect this would be readable
         tcp->IOHandler->ReadBytes(bufA, 255, false);    // Yes, this triggers an EIdReadTimeout Exception



In my humble opinion, code following Connect() should never be executed in cases where an EIdConnectTimeout Exception is thrown, this is certainly the case when the EIdConnectTimeout Exception is caught.

When an EIdConnectTimeout Exception is NOT caught, my Quota Non Page Pool Usage grows to a point where it is exhausted and crashes the system causing all kinds of inexplicable behavior in the application including Cannot write to canvas, No MDI available and others ....

I do hope that someone from the Indy Pit Crew can answer this and suggest a work-around until it is fixed.

I have carried out my tests confirming this a large number of times, the result is always the same but I am happy to be proven wrong if it can be shown that I have missed something.

Cheers
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm

Re: BUG in TIdTcpClient

Postby rlebeau » Thu Aug 25, 2016 8:44 pm

theLizard wrote:It is my opinion that there is a bug in TidTcpClient.


There is no such bug in Indy as you describe.

theLizard wrote:This bug relates to TidTcpClient->Connect() an EIdConnectTimeout Exception is NOT always triggered when there is NO physical connection to the network.


It is possible for exceptions other than EIdConnectTimeout to be to be raised by Connect() in the situation you have described.

theLizard wrote:Follow the code..


There are several issues to point out about Connect():

1. 200ms is too short a time for a meaningful connect timeout. Try something larger, at least several seconds. I usually use 5-10 seconds.

2. If you set the client's Host property to a hostname instead of an IP address, ConnectTimeout does not account for the time needed to resolve the hostname into an IP address. ConnectTimeout only applies to the actual connection attempt to an IP address. If you need a timeout on Host resolving, use TIdDNSRevolver before calling Connect().

3. Connect() uses an internal thread to implement the ConnectTimeout logic (this will change in the future). ConnectTimeout does not account for the time needed to create the thread, or the time needed to process window messages when Connect() is called in the main UI thread and TIdAntiFreeze is enabled, or the time spent task switching between threads.

So it is possible for Connect() to run longer than the ConnectTimeout specifies. ConnectTimeout is more of a hint than an absolute.

theLizard wrote:
Code: Select all
if(!tcp->Connected())



Did you verify that Connected() is returning false when the problem happens? Remember that Connected() can return true if there is unread data in the IOHandler's InputBuffer, even if the socket has been disconnected.

theLizard wrote:
Code: Select all
tcp->IOHandler->WriteDirect(writeA, 19, 0);  //I would also expect a timeout here



Not if Connect() was successfully connected to a server without error. By default, a socket buffers outgoing data and then sends it in the background (set the client's UseNagle property to false to disable that behavior). As long as the socket does not report an error buffering the data, WriteData() will not throw an exception.

theLizard wrote:
Code: Select all
if(tcp->IOHandler->Readable(500))            //I would NOT expect this would be readable
   tcp->IOHandler->ReadBytes(bufA, 255, false);    // Yes, this triggers an EIdReadTimeout Exception



It can, yes. Readable() returns true only if there is any data pending in the socket's inbound buffer, including disconnect notifications. So, if Readable() is true, there is *something* waiting to be read from the socket. But that does not guarantee that there are 255 bytes waiting to be read. ReadBytes() will not exit until either all 255 bytes have been read or an error has occurred, including a read timeout.

theLizard wrote:In my humble opinion, code following Connect() should never be executed in cases where an EIdConnectTimeout Exception is thrown, this is certainly the case when the EIdConnectTimeout Exception is caught.


It is not possible for Connect() to exit without raising *some* exception if a connection to the server could not be established. But that exception is NOT guaranteed to be EIdConnectTimeout. It could be EIdConnectException or EIdSocketError instead.

It is also possible for Connect() to successfully connect to a server and then be immediately disconnected on the other end before Indy performs any socket I/O. And if the disconnect is not graceful, errors will not be reported by the socket right away.

theLizard wrote:When an EIdConnectTimeout Exception is NOT caught, my Quota Non Page Pool Usage grows to a point where it is exhausted and crashes the system causing all kinds of inexplicable behavior in the application including Cannot write to canvas, No MDI available and others ....


I can guarantee you that there are no memory leaks in Indy's exceptions. However, that assumes that your compiler is catching thrown exceptions to begin with, otherwise it will leak them. That is not on Indy''s side.

For example, if you are using a Clang-based compiler, watch out for this gotcha:

Differences Between Clang-enhanced C++ Compilers and Previous-Generation C++ Compilers: Try Blocks Cannot Handle Some Exceptions

theLizard wrote:I do hope that someone from the Indy Pit Crew can answer this and suggest a work-around until it is fixed.


There is nothing to fix in Indy, because there is nothing broken on Indy's side. ConnectTimeout works, it just might not work in the time frame you are expecting, and it might not throw the type of exception you are expecting.
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1375
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: BUG in TIdTcpClient

Postby theLizard » Mon Aug 29, 2016 3:57 pm

rlebeau wrote:There is no such bug in Indy as you describe.


There may not be but, I cannot get past the fact that NO exception was thrown in 2 out of 10 connect attempts to a serial server when there was NO physical connection, you said in my other post,
rlebeau wrote:That should be physically impossible if the cable was not plugged in.


I said Exactly...

rlebeau wrote:It is possible for exceptions other than EIdConnectTimeout to be to be raised by Connect() in the situation you have described


Absolutely, but should these be caught on the catch( ... ) and not execute code past the point where the exception occurred or is this an appropriate behavior, I am guessing not.

rlebeau wrote:There are several issues to point out about Connect():

1. 200ms is too short a time for a meaningful connect timeout. Try something larger, at least several seconds. I usually use 5-10 seconds


5 - 10 Seconds is a long time to wait considering that a request for data is made every 1 second or less, maybe I am using the wrong component.

rlebeau wrote:2. If you set the client's Host property to a hostname instead of an IP address, ConnectTimeout does not account for the time needed to resolve the hostname into an IP address. ConnectTimeout only applies to the actual connection attempt to an IP address. If you need a timeout on Host resolving, use TIdDNSRevolver before calling Connect().


I am not sure why you gave this response, I have never used a hostname, nor have I suggested in any of my posts that I did,I only use IP addresses as a, xxx.xxx.xxx.xxx

rlebeau wrote:3. Connect() uses an internal thread to implement the ConnectTimeout logic (this will change in the future). ConnectTimeout does not account for the time needed to create the thread, or the time needed to process window messages
rlebeau wrote:Did you verify that Connected() is returning false when the problem happens? Remember that Connected() can return true if there is unread data in the IOHandler's InputBuffer, even if the socket has been disconnected.
when Connect() is called in the main UI
thread and TIdAntiFreeze is enabled, or the time spent task switching between threads.


In my case, Connect() is NOT called from any UI form, it is called within a thread that is created only when a connection to a serial server is called for.

rlebeau wrote:Did you verify that Connected() is returning false when the problem happens? Remember that Connected() can return true if there is unread data in the IOHandler's InputBuffer, even if the socket has been disconnected.


Input buffers are cleared before connection attempt is made, but yes, your statement here is of course correct, problem is that there could never have been a connection for the input buffers to contain data because there was no physical connection to the network for data to be returned, refer other post.

rlebeau wrote:Not if Connect() was successfully connected to a server without error. By default, a socket buffers outgoing data and then sends it in the background (set the client's UseNagle property to false to disable that behavior). As long as the socket does not report an error buffering the data, WriteData() will not throw an exception.


refer my other post where you said

rlebeau wrote:That should be physically impossible if the cable was not plugged in.


rlebeau wrote:It can, yes. Readable() returns true only if there is any data pending in the socket's inbound buffer, including disconnect notifications. So, if Readable() is true, there is *something* waiting to be read from the socket. But that does not guarantee that there are 255 bytes waiting to be read. ReadBytes() will not exit until either all 255 bytes have been read or an error has occurred, including a read timeout.


Readable() can only return true if a physical connection to the network exists and a some sort of response is received from the device data is being requested so, the problem here is that it got to this point in code when in fact it should never have reached it because there was NO connection to the network which as you say.

rlebeau wrote:That should be physically impossible if the cable was not plugged in.


rlebeau wrote:It is not possible for Connect() to exit without raising *some* exception if a connection to the server could not be established. But that exception is NOT guaranteed to be EIdConnectTimeout. It could be EIdConnectException or EIdSocketError instead.


Absolutely correct, that is why EVERY exception is caught, you taught me that lesson a long time ago and have made sure that errors of this nature are properly dealt with.

rlebeau wrote:It is also possible for Connect() to successfully connect to a server and then be immediately disconnected on the other end before Indy performs any socket I/O. And if the disconnect is not graceful, errors will not be reported by the socket right away.


No, it is NOT possible for Connect() to successfully connect to a server when there is NO physical connection to the network

rlebeau wrote:That should be physically impossible if the cable was not plugged in.


Exactly!!


I can accept that there maybe something on my system that gives the appearance of a connection (I don't know what it would be) but, I wont eliminate any possibility I will know more after I get my new system with new routers and new wifi cards totally isolated from any other network and repeat my tests, this will tell me if it is my system or if there is an Indy problem with TIdTcpClient.

rlebeau wrote:I can guarantee you that there are no memory leaks in Indy's exceptions. However, that assumes that your compiler is catching thrown exceptions to begin with, otherwise it will leak them. That is not on Indy''s side.

For example, if you are using a Clang-based compiler, watch out for this gotcha:


I have looked at this and I don't think that it applies in this case.
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm

Re: BUG in TIdTcpClient

Postby rlebeau » Mon Aug 29, 2016 5:09 pm

theLizard wrote:There may not be but, I cannot get past the fact that NO exception was thrown in 2 out of 10 connect attempts to a serial server when there was NO physical connection, you said in my other post,

rlebeau wrote:That should be physically impossible if the cable was not plugged in.


I said Exactly...


Unless the WinSock driver itself is faulty, it is not possible for the socket API connect() function to report success if it fails to establish a connection with a server. TIdTCPClient::Connect() raises an exception if the socket API connect() reports a failure. If there is no exception thrown, then the socket API connect() is not reporting a failure, it is connected to something. Check with TIdTCPClient's Socket->PeerIP and Socket->PeerPort properties after Connect() exits without exception to see what it is actually connected to.

theLizard wrote:
rlebeau wrote:It is possible for exceptions other than EIdConnectTimeout to be to be raised by Connect() in the situation you have described


Absolutely, but should these be caught on the catch( ... ) and not execute code past the point where the exception occurred or is this an appropriate behavior, I am guessing not.


In theory, catch(...) should be able to catch any thrown exception. In practice, at least in older compilers, I have seen it NOT always catch VCL-based exceptions correctly. So, for that reason alone, I always make sure to have a catch(const Exception&) block if I am expecting a VCL exception, and usually also have catch(...) as a fallback, just in case.

With the issue of asynchronous exceptions sometimes being ignored by CLang-based compilers, who knows how catch(...) behaves in the newer compilers.

theLizard wrote:5 - 10 Seconds is a long time to wait considering that a request for data is made every 1 second or less, maybe I am using the wrong component.


Well, you can't request data from the device until the connection is established, however long that actually takes. If your code requires data every 1 second, and there is no connection, what are you going to do? Why are you even waiting for data before the connection is established?

You will likely have to resort to using a background thread that maintains a persistent connection to the device and caches the latest data locally, reconnects when needed, and then the rest of your code can ask the thread for its latest cached data when needed.

theLizard wrote:
rlebeau wrote:2. If you set the client's Host property to a hostname instead of an IP address, ConnectTimeout does not account for the time needed to resolve the hostname into an IP address. ConnectTimeout only applies to the actual connection attempt to an IP address. If you need a timeout on Host resolving, use TIdDNSRevolver before calling Connect().


I am not sure why you gave this response, I have never used a hostname, nor have I suggested in any of my posts that I did,I only use IP addresses as a, xxx.xxx.xxx.xxx


I was merely pointing out that ConnectTimeout only applies to a small subset of what Connect() does internally. It does not apply to the entire Connect() operation as a while. Some people do assume that, and then wonder why it runs longer than requested.

theLizard wrote:Readable() can only return true if a physical connection to the network exists and a some sort of response is received from the device data is being requested so


If you re-read what I wrote earlier, I stated another possible way for Readable() to return true. For instance, if you successfully connect to something, and then get disconnected gracefully, that can also signal Readable().

theLizard wrote:the problem here is that it got to this point in code when in fact it should never have reached it because there was NO connection to the network which as you say.

rlebeau wrote:That should be physically impossible if the cable was not plugged in.



IF you were connecting to a remote IP, yes. But clearly you are connecting to something else, something that does not require a physical network connection to reach it, so you are probably connected to something on the local machine instead and not realizing it. Or maybe your machine has multiple network connections, and it is connecting to the device through a different network route than you are expecting? I don't know, since I don't know your network setup.

theLizard wrote:No, it is NOT possible for Connect() to successfully connect to a server when there is NO physical connection to the network


Clearly, it is connecting to something, or you would be getting an exception thrown instead.

theLizard wrote:I can accept that there maybe something on my system that gives the appearance of a connection (I don't know what it would be)


Have you stepped into Indy's source code with the debugger and watched how the socket API behaves inside of TIdTCPClient::Connect()?
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1375
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: BUG in TIdTcpClient

Postby theLizard » Mon Aug 29, 2016 11:03 pm

rlebeau wrote:Unless the WinSock driver itself is faulty, it is not possible for the socket API connect()


This is a distinct possibility because I am doubting my Windows 10 installation, my issues may have something to do with Windows 10, I will get a better idea when my new system arrives with Win10 Professional.

rlebeau wrote:In theory, catch(...) should be able to catch any thrown exception.

Agreed, in general as you have said, I too in most cases, use a catch(const Exception &e) in cases where there is some predictability and nothing drastic could take place i simple use catch( ... ) and ignore them (if they cause no issue of course)

rlebeau wrote:Well, you can't request data from the device until the connection is established, however long that actually takes.


Of course this is true, in my case however, waiting more than 1 second to connect is unacceptable as there will be other devices in the cue to consider and yes, I am aware that if one cannot connect in that time frame due to network congestion or other reasons the others may also fail, the idea is that attempts to connect will continue without really worrying about missed data so it is not critical that a connection is made at each request interval only that failures to connect a reported by timeouts, or other EId errors.

rlebeau wrote:If you re-read what I wrote earlier, I stated another possible way for Readable() to return true. For instance, if you successfully connect to something, and then get disconnected gracefully, that can also signal Readable().


yes, I understood this and realize that readability can alter between request for data and reading response.

rlebeau wrote:IF you were connecting to a remote IP, yes. But clearly you are connecting to something else, something that does not require a physical network connection to reach it, so you are probably connected to something on the local machine instead and not realizing it. Or maybe your machine has multiple network connections, and it is connecting to the device through a different network route than you are expecting? I don't know, since I don't know your network setup.


I hear what you are saying and understand that these are all possibilities as remote as they maybe, I do have a lan card on my system connected to a switch but these are not connecting to anything outside of my office environment (printers) and it cannot connect to the device in question.

rlebeau wrote:Clearly, it is connecting to something, or you would be getting an exception thrown instead.


Agree, something is going on.

rlebeau wrote:Have you stepped into Indy's source code with the debugger and watched how the socket API behaves inside of TIdTCPClient::Connect()?


Yes, I did step through some Indy code when a exception occurred but I am not sure that I got into the Connect part., will try and see what happens there.
theLizard
BCBJ Master
BCBJ Master
 
Posts: 439
Joined: Wed Mar 18, 2009 2:14 pm


Return to Technical

Who is online

Users browsing this forum: Bing [Bot] and 12 guests