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: 449
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: 1528
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: 449
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: 1528
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: 449
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: 449
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: 1528
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: 449
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: 1528
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: 449
Joined: Wed Mar 18, 2009 2:14 pm

Re: TIdTcpClient - Memory Leak

Postby corbingravely » Mon Jan 29, 2018 10:19 am

This is something that i found in another forum.. Hope this helps.....
-----------------------------------------------
It looks like TIdSMTP is leaking memory is created at run time in C++.

There are no known leaks in TIdSMTP (or any other Indy component, for that
matter).

Each click on the button will increase memory usage (as reported
by task manager).

You cannot use TaskManager to test for memory leaks. TM has no concept of
HOW your app uses memory, only HOW MUCH memory has been allocated to the
app. Keep in mind that Delphi/C++Builder's memory manager caches freed memory
for later reuse, the memory is not returned to the OS. If TM reports memory
usage is growing, that is not always an indication of a real leak. More
likely you are just fragmenting memory instead, causing the memory manager
to be unable to reuse memory it already has so it asks the OS for more memory.
To properly test for leaks, you have to let the memory manager report
the leaks to you, since only it knows what constitutes a real leak. Set
the global System::ReportMemoryLeaksOnShutdown variable (http://docwiki.embarcadero.com/Librarie ... OnShutdown)
to true, or else install the full version of FastMM (http://fastmm.sourceforge.net)
into your project and enable its leak reporting capabilities.

In addition, madExcept also report a memory leak in line 645 of
IdMessageClient.pas:
FMsgLineFold := TAB;

FMsgLineFold is a String member of the TIdMessageClient class.

In VCL, the only way that member can leak is if the TIdMessageClient destructor
is not being called. TIdSMTP derives from TIdMessageClient, but you are
destroying your TIdSMTP objects, so that could not be a leak. And every
place where Indy internally creates a TIdMessageClient object is protected
by a try/finally block to ensure the object is destroyed. So those cannot
be a leak, either. If MadExcept is claiming that FMsgLineFold is leaking
then it has to also be reporting that the owning TIdMessageClient is also
leaking, and it should be showing you the call stack where that object is
being created.

By chance, are you compiling this code in a FireMonkey mobile app? The code
you showed WOULD "leak" the objects (at least until the Form itself is destroyed)
because of ARC handling. The 'delete' operator does not destroy a TObject-derived
object under ARC, it merely decrements the object's reference count. You
are assigning an Owner to the TIdSMTP objects, and the Owner would have active
references to the TIdSMTP objects, thus your 'delete' would not have an effect.
To remove the active references, you would have to either:

1. not assign an Owner in the first place:

Code: Select all
void __fastcall OnClick(Sender...)
{
    TIdSMTP * SMTP;
    int i;
    for(i=0;i<100000;i++)
    {
        SMTP = new TIdSMTP(NULL);
        delete SMTP;
    }
}


2. remove the TIdSMTP objects from the Owner:
Code: Select all
void __fastcall OnClick(Sender...)
{
    TIdSMTP * SMTP;
    int i;
    for(i=0;i<100000;i++)
    {
        SMTP = new TIdSMTP(this);
        this->RemoveComponent(SMTP);
        delete SMTP;
    }
}



----------------------------------------------
Corbin Gravely
corbingravely
 
Posts: 9
Joined: Tue Jan 21, 2014 11:08 pm

Re: TIdTcpClient - Memory Leak

Postby rlebeau » Mon Jan 29, 2018 1:18 pm

corbingravely wrote:This is something that i found in another forum.. Hope this helps.....


Looks like something I wrote...

corbingravely wrote:...
To remove the active references, you would have to either:

1. not assign an Owner in the first place:

2. remove the TIdSMTP objects from the Owner:


A 3rd option is to call the object's DisposeOf() method (or use Indy's IdDisposeAndNil() function):

Code: Select all
void __fastcall OnClick(Sender...)
{
    TIdSMTP * SMTP;
    int i;
    for(i=0;i<100000;i++)
    {
        SMTP = new TIdSMTP(this);
        SMTP->DisposeOf(); // IdDisposeAndNil(SMTP);
    }
}
Remy Lebeau (TeamB)
Lebeau Software
User avatar
rlebeau
BCBJ Author
BCBJ Author
 
Posts: 1528
Joined: Wed Jun 01, 2005 3:21 am
Location: California, USA


Return to Technical

Who is online

Users browsing this forum: Google [Bot] and 14 guests