TIdTcpClient - Memory Leak

This is the forum for miscellaneous technical/programming questions.

Moderator: 2ffat

TIdTcpClient - Memory Leak

Postby theLizard » Wed Aug 10, 2016 11:41 pm

I have a memory leak problem that I do not understand, I have a TIdTcplclient in a thread, I connect and disconnect without a problem.

Stepping through the code all seems to be fine BUT Quota Peak Non Paged Pool Usage and Quota Non Paged Pool Usage increases rapidly, if there is not connection to my router, Any thoughts?

Is there any check I can do to determine if there is an actual connection.

Code: Select all
  try
    {
     try
      {
      if(!tcp->Connected())
         tcp->Connect();

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

      tcp->IOHandler->WriteDirect(writeA, 19, 0);

      if(tcp->IOHandler->Readable(500))
         tcp->IOHandler->ReadBytes(bufA, 255, false);

      tcp->IOHandler->InputBuffer->Clear();
      tcp->IOHandler->WriteDirect(writeB, 19, 0);

      if(tcp->IOHandler->Readable(500))
         tcp->IOHandler->ReadBytes(bufB, 255, false);

      tcp->IOHandler->WriteLn(v4->Send->EndRequest);

      tcp->Disconnect();

      dataOk = true;
      finished = true;
      }
   catch(const Exception &e )
      {
      finished = true;
      dataOk = false;
      if(e.Message != NULL)
         {
         if (dynamic_cast<const EIdConnClosedGracefully *>(&e) != NULL)
            {
            }
         if (dynamic_cast<const EIdReadTimeout *>(&e) != NULL)
            {
            }
         else
            if (dynamic_cast<const EIdException *>(&e) != NULL)
               {
               }
         }
      tcp->Disconnect(); 
      }
   }
__finally
   {
   if(dataOk)
      Synchronize(&Result);
   else
      Synchronize(&BadData);
   }

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

Re: TIdTcpClient - Memory Leak

Postby rlebeau » Thu Aug 11, 2016 8:01 pm

theLizard wrote:Stepping through the code all seems to be fine BUT Quota Peak Non Paged Pool Usage and Quota Non Paged Pool Usage increases rapidly, if there is not connection to my router, Any thoughts?


I don't know if those particular values are indicative of a real memory leak. Things like the Working Set would, though. Try looking at that instead.

theLizard wrote:Is there any check I can do to determine if there is an actual connection.


The only reliable method is to connect and handle an error if it occurs.

Unless the OS knows for sure that a connection has been closed, anything else is guessing based on incomplete information. A dead socket closed incorrectly might not be invalidated by the OS for awhile, so any reads (including TIdIOHandler.Connected()) will simply act like 0 bytes are currently available, and any sends will be cached in the socket's internal buffer until it fills up.

While your code is technically OK, I would suggest not calling WriteDirect() directly, and not calling Readable() at all. Use the ReadTimeout property instead (500ms is a pretty small timeout, though).

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

    tcp->Connect();
    try
    {
        tcp->IOHandler->Write(writeA, 19, 0);
        tcp->IOHandler->ReadBytes(bufA, 255, false);

        tcp->IOHandler->Write(writeB, 19, 0);
        tcp->IOHandler->ReadBytes(bufB, 255, false);

        tcp->IOHandler->WriteLn(v4->Send->EndRequest);
    }
    __finally {
        tcp->Disconnect();
    }

    dataOk = true;
}
catch (const Exception &e)
{
    dataOk = false;

    if (dynamic_cast<const EIdConnClosedGracefully *>(&e) != NULL)
    {
        //...
    }
    else if (dynamic_cast<const EIdReadTimeout *>(&e) != NULL)
    {
         //...
    }
    else if (dynamic_cast<const EIdException *>(&e) != NULL)
    {
        //...
    }
}

finished = true;

if (dataOk)
    Synchronize(&Result);
else
    Synchronize(&BadData);
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1387
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: TIdTcpClient - Memory Leak

Postby theLizard » Thu Aug 11, 2016 11:54 pm

rlebeau wrote:I don't know if those particular values are indicative of a real memory leak. Things like the Working Set would, though. Try looking at that instead.


There is a relationship between Quota Non Page Pool Usage and my issues.

Working Set memory is being managed by periodically by calling EmptyWorkingSet(GetCurrentProcess());

rlebeau wrote:The only reliable method is to connect and handle an error if it occurs.


I certainly thought the same, however, errors are not always triggered as my tests have concluded.

rlebeau wrote:Unless the OS knows for sure that a connection has been closed, anything else is guessing based on incomplete information. A dead socket closed incorrectly might not be invalidated by the OS for awhile, so any reads (including TIdIOHandler.Connected())


Agreed, but, when there is no physical connection to the endpoint, the OS knows this but TIdTcpClient does not in all cases seem to know and proceeds as if it has made a connection to an endpoint that does not exist.

rlebeau wrote:I would suggest not calling WriteDirect()


Unfortunately, (not that I mean it is unfortunate) but using WriteLn or other similar cannot be used, because the number of bytes sent are 2 more than what I want sent (cr/lf) in WriteLn for instance.

rlebeau wrote:and not calling Readable() at all


Agreed, this should not be necessary and was not, only put in to see what was going on, it to says buffers are readable where in fact there is NO physical connection to an endpoint because there is no connection to the outside world from the system the tests are being carried out on.

I can say for certain that the issues I am having is network related and TIdTcpClient appears to be the culprit in this instance.

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

Re: TIdTcpClient - Memory Leak

Postby rlebeau » Sat Aug 13, 2016 11:32 pm

theLizard wrote:Working Set memory is being managed by periodically by calling EmptyWorkingSet(GetCurrentProcess());


That is absolutely the wrong thing to be doing. It means you are mismanaging memory and just putting a bandaid on it instead of fixing the root problem.

theLizard wrote:Agreed, but, when there is no physical connection to the endpoint, the OS knows this


Yes, but not necessarily in a timely manner. Remember, TCP is actually designed to survive network outages. In the absence of signals to indicate the end of the connection (like FIN packets), it can take time for the OS to time out and invalidate an open socket. Yanking out the cable is not always enough. In fact, once upon a time, Windows did not react to yanked cables. You could yank a cable and put it back on, and Windows happily kept the connection alive. Modern Windows are better about reacting to yanked cables, but it is still not always guaranteed.

theLizard wrote:but TIdTcpClient does not in all cases seem to know and proceeds as if it has made a connection to an endpoint that does not exist.


If Indy is not the one closing the connection, it does not know the connection is gone until socket API calls report it. Either when FIN is received, or the OS times out and stops accepting new operations on behalf of the connection.

theLizard wrote:Unfortunately, (not that I mean it is unfortunate) but using WriteLn or other similar cannot be used, because the number of bytes sent are 2 more than what I want sent (cr/lf) in WriteLn for instance.


You can use `Write()` instead of `WriteLn()`. Just don't use `WriteDirect()`, it is not meant to be used directly, it is an internal call.

theLizard wrote:I can say for certain that the issues I am having is network related and TIdTcpClient appears to be the culprit in this instance.


I don't deny that you might be having network related problems, but I can say for certain that it is not Indy's fault, and it certainly does not leak memory the way you have described. Something else is going on.
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1387
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: TIdTcpClient - Memory Leak

Postby theLizard » Sun Aug 14, 2016 12:00 am

rlebeau wrote:That is absolutely the wrong thing to be doing. It means you are mismanaging memory and just putting a bandaid on it instead of fixing the root problem.


Apart from the fact I had unexplained memory usage that ONLY occurred with TIdTcpClient behaving in a way that I could NOT trace into (including unexplained random lock up of the app) and that I wnet through every bit of code that may have been a source of memory leak and not finding anything, I put this extream measuer in an effort to find may be going on, I absolutely agree that needing to do this (and by the way, working set memory was not the problem anyway) is not the way to go about fixing a problem.

rlebeau wrote:Yes, but not necessarily in a timely manner. Remember, TCP is actually designed to survive network outages.


Again, I will agree on this however, stepping through the code showed a problem, whether it is TIdTcpClient or something else, including the device which is being polled is still a mystery.

I can report however that I appear to have solved the problem in an unusual way which I am sure that you would not agree with so I wont go into what I did but I appear to have stability, very few errors and NO (apparent) memory leaks so far.

I can also tell you that outside of the Berlin IDE, the app is running very smoothly, NO lock ups and doing exactly as it was designed to do, thread management is working like a dream,

rlebeau wrote:f Indy is not the one closing the connection,


Problem here is that I could not identify who or what was causing the problem, part of the Page Pool Usage was found within windows itself.

At the end of the day, the problem "seems" to be solved with the work around (fingers crossed) and I can get back to moving on with other things.

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

Re: TIdTcpClient - Memory Leak

Postby theLizard » Sun Aug 14, 2016 6:34 pm

rlebeau wrote:If Indy is not the one closing the connection, it does not know the connection is gone until socket API calls report it. Either when FIN is received, or the OS times out and stops accepting new operations on behalf of the connection.


This slipped by me so will address this now.

Points to remember
1 - Network was physically fully disconnected before the app was started.
2 - Pinging the ip address of the device resulted in "Request Timed out" or "Destination host unreachable"
3 - 8 out of 10 times TidTcpClient threw a EIdConnectTimeout the other 2 times it assumed a connection was made and continued on as if there were no problems connecting.

Question is, is this normal behavior for TIdTcpClient, I am guessing not.

If the network was disconnected in the manner described, how can anything disconnect TIdTcpClient when there was no way it could have been connected in the first place.

I may be missing something and I agree that TIdTcpClient can not know if the connection is gone until the the OS says otherwise but should TIdTcpClient know from the OS that a connection had been made!!

I am just trying to get my head around this with some sort of basic understanding of how TIdTcpClient interacts with the OS to connect and disconnect it's services.

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

Re: TIdTcpClient - Memory Leak

Postby rlebeau » Mon Aug 15, 2016 3:26 pm

theLizard wrote: 3 - 8 out of 10 times TidTcpClient threw a EIdConnectTimeout the other 2 times it assumed a connection was made and continued on as if there were no problems connecting.


That should be physically impossible if the cable was not plugged in. The lower level socket API connect() function would never succeed, it would report an error, causing Indy's higher-level TIdTCPClient.Connect() method to raise an exception. There is no way for TIdTCPClient.Connect() to exit without error if the socket API connect() failed. See the implementation in TIdIOHandlerStack.ConnectClient().

theLizard wrote:I may be missing something and I agree that TIdTcpClient can not know if the connection is gone until the the OS says otherwise but should TIdTcpClient know from the OS that a connection had been made!!


Yes, and it does, by virtue of the fact that the socket API connect() function either succeeds or fails.
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1387
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: TIdTcpClient - Memory Leak

Postby theLizard » Mon Aug 15, 2016 5:10 pm

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


Exactly, yet, what I have described is 100% correct, what I cannot determine is what maybe the cause of this.

The tests were carried out numerous times with the same result, I have looked at (almost) everything relating to TIdTcpClient, it's IOHandler and possible alternatives, I have NO doubt that my code is correct in the way it assigns port and host properties, checks for errors and ensuring that NO UI is accessed in the reading thread outside of Synchronize.

I have also carried out other tests including using WinSock
Code: Select all
IcmpSendEcho(hIcmpFile, ipaddr, SendData, sizeof(SendData), NULL, ReplyBuffer, ReplySize, 1000);


It of course returns "Host Unreachable" if it cannot get to the end point.

If it's not Indy, it's something in my system or IDE (after all, it steps into code that it should not)

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

Re: TIdTcpClient - Memory Leak

Postby rlebeau » Mon Aug 15, 2016 7:54 pm

theLizard wrote:Exactly, yet, what I have described is 100% correct, what I cannot determine is what maybe the cause of this.


Can you reproduce it using WinSock API calls directly (WSAStartup(), socket(), bind(), connect(), etc) without using Indy?

theLizard wrote:I have also carried out other tests including using WinSock
Code: Select all
IcmpSendEcho(hIcmpFile, ipaddr, SendData, sizeof(SendData), NULL, ReplyBuffer, ReplySize, 1000);



IcmpSendEcho() is not part of WinSock, it is part of the IP Helper API.
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1387
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA

Re: TIdTcpClient - Memory Leak

Postby theLizard » Mon Aug 15, 2016 11:45 pm

theLizard wrote:IcmpSendEcho() is not part of WinSock, it is part of the IP Helper API.

My bad, it is one of the Ping tests I did from within the application, of coyurse there was other code associated with it.

rlebeau wrote:Can you reproduce it using WinSock API calls directly (WSAStartup(), socket(), bind(), connect(), etc) without using Indy?


No, I created a test app using WinSock API iResult = WSAStartup(MAKEWORD(2,2), &wsaData);

When not connected I get invalid handles using the following example code from Microsoft , when connected I get data back from the device sometimes in full other times it gets stuck on iResult = recv(ConnectSocket, recvbuf, recvbuflen, 0); in the do .. while loop

Code: Select all
      WSADATA wsaData;
      SOCKET ConnectSocket = INVALID_SOCKET;
      struct addrinfo *result = NULL,
                              *ptr = NULL,
                              hints;
      char *sendbuf = "/?00030000019400!\r\nx020‰ä\x02\x01";
      char recvbuf[DEFAULT_BUFLEN];
      int iResult;
      int recvbuflen = DEFAULT_BUFLEN;

      char ip[] = ".....";

      // Initialize Winsock
      iResult = WSAStartup(MAKEWORD(2,2), &wsaData);
      if (iResult != 0) {
            printf("WSAStartup failed with error: %d\n", iResult);
            return ;
      }

      ZeroMemory( &hints, sizeof(hints) );
      hints.ai_family = AF_UNSPEC;
      hints.ai_socktype = SOCK_STREAM;
      hints.ai_protocol = IPPROTO_TCP;

      // Resolve the server address and port
      iResult = getaddrinfo(ip, DEFAULT_PORT, &hints, &result);
      if ( iResult != 0 ) {
            printf("getaddrinfo failed with error: %d\n", iResult);
            WSACleanup();
            return ;
      }

      // Attempt to connect to an address until one succeeds
      for(ptr=result; ptr != NULL ;ptr=ptr->ai_next) {

            // Create a SOCKET for connecting to server
            ConnectSocket = socket(ptr->ai_family, ptr->ai_socktype,
                  ptr->ai_protocol);
            if (ConnectSocket == INVALID_SOCKET) {
                  printf("socket failed with error: %ld\n", WSAGetLastError());
                  WSACleanup();
                  return ;
            }

            // Connect to server.
            iResult = connect( ConnectSocket, ptr->ai_addr, (int)ptr->ai_addrlen);
            if (iResult == SOCKET_ERROR) {
                  closesocket(ConnectSocket);
                  ConnectSocket = INVALID_SOCKET;
                  continue;
            }
            break;
      }

      freeaddrinfo(result);

      if (ConnectSocket == INVALID_SOCKET) {
            printf("Unable to connect to server!\n");
            WSACleanup();
            return ;
      }

      // Send an initial buffer
      iResult = send( ConnectSocket, sendbuf, (int)strlen(sendbuf), 0 );
      if (iResult == SOCKET_ERROR) {
            printf("send failed with error: %d\n", WSAGetLastError());
            closesocket(ConnectSocket);
            WSACleanup();
            return ;
      }

      printf("Bytes Sent: %ld\n", iResult);

      // shutdown the connection since no more data will be sent
      iResult = shutdown(ConnectSocket, SD_SEND);
      if (iResult == SOCKET_ERROR) {
            printf("shutdown failed with error: %d\n", WSAGetLastError());
            closesocket(ConnectSocket);
            WSACleanup();
            return ;
      }

      // Receive until the peer closes the connection
      do {

            iResult = recv(ConnectSocket, recvbuf, recvbuflen, 0);
            if ( iResult > 0 )
                  printf("Bytes received: %d\n", iResult);
            else if ( iResult == 0 )
                  printf("Connection closed\n");
            else
                  printf("recv failed with error: %d\n", WSAGetLastError());

      s = LastError();

      } while( iResult > 0 );


      m->Lines->Add(s);

m->Lines->Add(recvbuf);

   sendbuf = "\x01\x42\x30\x03\x75";

      iResult = send( ConnectSocket, sendbuf, (int)strlen(sendbuf), 0 );
      if (iResult == SOCKET_ERROR) {
            printf("send failed with error: %d\n", WSAGetLastError());
            closesocket(ConnectSocket);
            WSACleanup();
            return ;
      }

      // cleanup
      closesocket(ConnectSocket);
      WSACleanup();



At this point I will not rule out anything other than there were no physical connection to my network from the development machine where these tests are being conducted.

my wifi is a NETGEAR WNA3100 USB stick which connects to a NETGEAR WNR2000 router (when a physical connection is established)

Drivers may be a source of the problem but I cannot determine this.

Cheers
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: No registered users and 4 guests

cron