FreeRTOS TCP/IP Stack possible memory leak?

Libraries
1

Hello,
I am using FreeRTOS TCP IP stack in my software to create a telnet TCP server. I discoverred that when ever I connect and disconnect a client I end up with fewer heap memory as before.
I monitored it using the vApplicationIdleHook() by printing out the current and minimum ever heap consumption to the console, whilest repeatedly connecting & disconnecting the client.

As a result I discoverred that a socket needs always 17.544 bytes of memory and I’m always missing 64 bytes after a re-connection.

Does somebody know if this may be related to the TCP/IP stack? I looked through my own code and could not find anything explaining the loss of these 64 bytes…

I’ll post the source code for accepting a new client below:

static void createTCPServerSocket( void *pvParameters )
{
	BaseType_t retVal_os = 0;
	uport_ctrl_t * pCtrl = (uport_ctrl_t *) pvParameters;
	TickType_t xSocketTimeOut = portMAX_DELAY;
	const BaseType_t xBacklog = pCtrl->numOfClients;
	socklen_t xSize = sizeof( pCtrl->sockAdrListener );
	event_client_accept_e queueIpClientCon;
	
	//Open new listening socket
	//***************************************
	/* Attempt to open the socket. */
	pCtrl->socketListener = FreeRTOS_socket( FREERTOS_AF_INET, FREERTOS_SOCK_STREAM, FREERTOS_IPPROTO_TCP );
	if(pCtrl->socketListener == FREERTOS_INVALID_SOCKET)
	{
		//Send feedback to parent
		queueIpClientCon = eErrIpStack;
		xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

		//Auto-delete this task now as it got useless
		vTaskDelete( NULL );
	}

	// Define for how long accept() will wait for a connection.
	retVal_os = FreeRTOS_setsockopt( pCtrl->socketListener,
									 0,		//currently unused parameter
									 FREERTOS_SO_RCVTIMEO,
									 &xSocketTimeOut,
									 sizeof(TickType_t) );

	if(retVal_os != STACK_SUCCESS)
	{		
		//Send feedback to parent
		queueIpClientCon = eErrIpStack;
		xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

		//Auto-delete this task now as it got useless
		vTaskDelete( NULL );
	}


	// Set the listening port
	pCtrl->sockAdrListener.sin_port = ( uint16_t ) pCtrl->port;
	pCtrl->sockAdrListener.sin_port = FreeRTOS_htons( pCtrl->sockAdrListener.sin_port );

	// Bind the socket to the port that the client RTOS task will send to.
	retVal_os = FreeRTOS_bind( pCtrl->socketListener, &pCtrl->sockAdrListener, sizeof( struct freertos_sockaddr) );


	if(retVal_os != STACK_SUCCESS)
	{
		//Send feedback to parent
		queueIpClientCon = eErrIpStack;
		xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

		//Auto-delete this task now as it got useless
		vTaskDelete( NULL );
	}

	// Set the socket into a listening state so it can accept connections.
	// The maximum number of simultaneous connections is limited to xBacklog.
	retVal_os = FreeRTOS_listen( pCtrl->socketListener, xBacklog );

	if(retVal_os != STACK_SUCCESS)
	{
		//Send feedback to parent
		queueIpClientCon = eErrIpStack;
		xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

		//Attempt graceful shutdown client socket
		FreeRTOS_shutdown( pCtrl->socketClient, FREERTOS_SHUT_RDWR );
		
		//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
		retVal_os = FreeRTOS_issocketconnected(pCtrl->socketClient);

		if(retVal_os == pdTRUE)
		{
			//Shutdown is complete and the socket can be safely closed.
			FreeRTOS_closesocket( pCtrl->socketClient );
		}
		
		//Attempt graceful shutdown listener socket
		FreeRTOS_shutdown( pCtrl->socketListener, FREERTOS_SHUT_RDWR );

		//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
		retVal_os = FreeRTOS_issocketconnected(pCtrl->socketListener);

		if(retVal_os == pdTRUE)
		{
			//Shutdown is complete and the socket can be safely closed.
			FreeRTOS_closesocket( pCtrl->socketListener );
		}
		
		//Auto-delete this task now as it got useless
		vTaskDelete( NULL );
	}

	//Create a temporary Socket that holds the newly connected client just until the old connection has been shutdown
	Socket_t tempSocket;

	while(retVal_os == STACK_SUCCESS)
	{
		/* Wait for incoming connections. */
		tempSocket = FreeRTOS_accept( pCtrl->socketListener, &pCtrl->sockAdrClient, &xSize);

		if(tempSocket == FREERTOS_INVALID_SOCKET )
		{
			//Send feedback to parent
			queueIpClientCon = eErrIpStack;
			xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

			//Attempt graceful shutdown client socket
			FreeRTOS_shutdown( pCtrl->socketClient, FREERTOS_SHUT_RDWR );
			
			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketClient);

			if(retVal_os == pdTRUE)
			{
				//Shutdown is complete and the socket can be safely closed.
				FreeRTOS_closesocket( pCtrl->socketClient );
			}
			
			//Attempt graceful shutdown listener socket
			FreeRTOS_shutdown( pCtrl->socketListener, FREERTOS_SHUT_RDWR );

			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketListener);

			if(retVal_os == pdTRUE)
			{
				//Shutdown is complete and the socket can be safely closed.
				FreeRTOS_closesocket( pCtrl->socketListener );
			}
			
			//Auto-delete this task now as it got useless
			vTaskDelete( NULL );
		}

		/* Shutdown the old client connection */
		//Make sure that the old socket does not point to the new one which would lead to closing the new one!
		//And make sure that there is no old socket as this would lead to closing a non-existent socket which in turn would blow the tcp/ip stack
		if(pCtrl->socketClient != tempSocket && pCtrl->socketClient != NULL)
		{
			FreeRTOS_shutdown( pCtrl->socketClient, FREERTOS_SHUT_RDWR );

			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketClient);

			if(retVal_os == pdTRUE)
			{
				FreeRTOS_closesocket( pCtrl->socketClient );
			}
		}

		/* Assign the new connection to the control struct */
		pCtrl->socketClient = tempSocket;

		//Send Queue that says that uportOpen() that a new client has been accepted
		queueIpClientCon = eClientConnected;

		// Create the set of sockets that will be passed into FreeRTOS_select().
		pCtrl->socketSet = FreeRTOS_CreateSocketSet();

		if(pCtrl->socketSet == NULL)
		{
			//Send feedback to parent
			queueIpClientCon = eErrIpStack;
			xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

			//Attempt graceful shutdown client socket
			FreeRTOS_shutdown( pCtrl->socketClient, FREERTOS_SHUT_RDWR );
			
			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketClient);

			if(retVal_os == pdTRUE)
			{
				//Shutdown is complete and the socket can be safely closed.
				FreeRTOS_closesocket( pCtrl->socketClient );
			}
			
			//Attempt graceful shutdown listener socket
			FreeRTOS_shutdown( pCtrl->socketListener, FREERTOS_SHUT_RDWR );

			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketListener);

			if(retVal_os == pdTRUE)
			{
				//Shutdown is complete and the socket can be safely closed.
				FreeRTOS_closesocket( pCtrl->socketListener );
			}
			
			//Auto-delete this task now as it got useless
			vTaskDelete( NULL );
		}

		//Set up sockets
		// Add the created socket to the set for the READ event.
		FreeRTOS_FD_SET( pCtrl->socketClient, pCtrl->socketSet, eSELECT_READ );

		//Add the created socket to the set for EXEPTIONAL events.
		FreeRTOS_FD_SET( pCtrl->socketClient, pCtrl->socketSet, eSELECT_EXCEPT );


		//Set receive Timeout to "no wait" as we will use select to pend on this socket
		xSocketTimeOut = 0;

		// Define for how long recv() will wait for a data.
		retVal_os = FreeRTOS_setsockopt( pCtrl->socketClient,
										 0,//currently unused parameter
										 FREERTOS_SO_RCVTIMEO,
										 &xSocketTimeOut,
										 sizeof(TickType_t) );

		// Define for how long send() will wait to send data.
		retVal_os |= FreeRTOS_setsockopt( pCtrl->socketClient,
										 0,//currently unused parameter
										 FREERTOS_SO_SNDTIMEO,
										 &xSocketTimeOut,
										 sizeof(TickType_t) );

		if(retVal_os != STACK_SUCCESS)
		{
			//Send feedback to parent
			queueIpClientCon = eErrIpStack;
			xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

			//Attempt graceful shutdown client socket
			FreeRTOS_shutdown( pCtrl->socketClient, FREERTOS_SHUT_RDWR );
			
			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketClient);

			if(retVal_os == pdTRUE)
			{
				//Shutdown is complete and the socket can be safely closed.
				FreeRTOS_closesocket( pCtrl->socketClient );
			}
			
			//Attempt graceful shutdown listener socket
			FreeRTOS_shutdown( pCtrl->socketListener, FREERTOS_SHUT_RDWR );

			//Sometimes FreeRTOS_shutdown() closes the socket right away, sometimes not - make sure the socket still exists before calling FreeRTOS_closesocket() to avoid an assert error
			retVal_os = FreeRTOS_issocketconnected(pCtrl->socketListener);

			if(retVal_os == pdTRUE)
			{
				//Shutdown is complete and the socket can be safely closed.
				FreeRTOS_closesocket( pCtrl->socketListener );
			}
			
			//Auto-delete this task now as it got useless
			vTaskDelete( NULL );
		}

		//Signal in control block that a new client connection is present
		pCtrl->bClientConnectionUp = true;


		//Send back to caller the status of the connection if something changed - either success or error without terminating this task though
		xQueueSend(pCtrl->osQueue_clientAccept, &queueIpClientCon, 0);

	} //while


	//Auto-delete this task now
	vTaskDelete( NULL );
}

Hopefully somebody know something about it.

Best Regards,

2

Hello @Stonebull,
Thank you for the code snippet. That is helpful.

Maybe unrelated to what you have asked, are you sure that you are closing the accepted socket?

I see that you:

  • accept a socket
    /* Wait for incoming connections. */
  	tempSocket = FreeRTOS_accept( pCtrl->socketListener, &pCtrl->sockAdrClient, &xSize);
  • set socket options for that socket
        // Define for how long recv() will wait for a data.
  	retVal_os = FreeRTOS_setsockopt( ... );

  	// Define for how long send() will wait to send data.
  	retVal_os |= FreeRTOS_setsockopt( ... );

But you never seem to close that socket in that cycle. You wait for another accept event and then close the old client socket. Is that what you intend?

If that is what you intend to do and doesn’t explain the issue, it will be helpful to know:

  • which platform are you using
  • is this error repetitive? I mean to say that if you continue accepting and closing a client connection over and over, do you eventually run out of memory?
3

I haven’t tried your implementation of a telnet server, but I tested my own server, just to see if there is a memory leak in the library.

If you like, here is a simple implementation of telnet: telnet.c/telnet.h.

This solution does not create any task, it wants to be called by the application.

Here is an example of how to use the telnet module:

#include "telnet.h"

#if ( ipconfigSOCKET_HAS_USER_SEMAPHORE == 0 )
    #error Please define ipconfigSOCKET_HAS_USER_SEMAPHORE as '1'
#endif

static Telnet_t xTelnet;
static SemaphoreHandle_t xServerSemaphore;

void my_application( void *pvParameter )
{
    char pcBuffer[ 128 ];

    xServerSemaphore = xSemaphoreCreateBinary();
    configASSERT( xServerSemaphore != NULL );

    xTelnetCreate( &xTelnet, TELNET_PORT_NUMBER );

    if( xTelnet.xParentSocket != 0 )
    {
        FreeRTOS_setsockopt( xTelnet.xParentSocket,
                             0,
                             FREERTOS_SO_SET_SEMAPHORE,
                             ( void * ) &xServerSemaphore,
                             sizeof( xServerSemaphore ) );
    }

    for( ;; )
    {
        /* Go asleep for at most 200 msec, the Telnet socket might give
		 * to the semaphore and wake-up this task.
         */
        xSemaphoreTake( xServerSemaphore, pdMS_TO_TICKS( 200U ) );

        if( xTelnet.xParentSocket != NULL )
        {
            struct freertos_sockaddr fromAddr;
			BaseType_t xCount;

            xCount = xTelnetRecv( &xTelnet,
                                  &fromAddr,
                                  pcBuffer,
                                  sizeof( pcBuffer ) - 1 );

            if( xCount > 0 )
            {
                pcBuffer[ xCount ] = 0;
                /* Just bounce back the string. */
                xTelnetSend( &xTelnet, &fromAddr, pcBuffer, xCount );
            }
        }
    }
}

Some general remarks, after reading your sample code:

Make sure that after calling closesocket(), the socket is not referred to any more. It is a good habit to clear the pointer:

    FreeRTOS_closesocket( pCtrl->socketClient );
    pCtrl->socketClient = NULL;

This might avoid an ugly crash.

Quite recently I wrote about the use of shutdown, when to use it and how. See my long post in this forum.

Normally, a TCP server will not shut down a connection. It is the client who initiates a graceful shutdown. The client is e.g. a telnet client or a browser.

Shutdown is applied to a connected socket, and after calling shutdown(), the socket must continue to exist because shutting down a connection takes time. On a LAN it will take a few milliseconds, but on the Internet it make take up to a few seconds.

Normally, the listening socket is created at boot time, and it continues to exist until the device goes down.
The client sockets obtained with accept() have a temporary life. use them until recv() or send() returns an error.

4

Hello kanherea, thanks for your reply!
I know my implementation may looks a bit strange and over complicated at first glance. I started with a simpler server implementation but kept adding to it to avoid several issues.

The most important criteria that my server needs to satisfy is to be always available for a client to connect and at the same time allow only one client at a time.
At first I implemented a server that allows only one client and re-uses the listener socket. But then I experienced issues when the client crashed for some reason and the connection was not closed properly as the server kept the connection alive for a long time.

I tried playing around with the ipconfigTCP_KEEP_ALIVE_INTERVAL but somehow it did not prevent the server from being unreachable for an unacceptable period of time.

By allowing two clients I make sure that once one client is connected I still can allow a new client to connect immediately. Once a new client is connected I kick the old one to have again a free child left.

Regarding your question about closing a socket, let me explain that with a little listing of events:

  1. As soon as the task is created the listening socket is set up.
  2. Then the program enters the while loop that accepts new clients
  3. As soon as a new client is accepted it is assigned to the tempSocket.
  4. Now the tempSocket is compared to the socketClient (which still is NULL) thus unequal so this if branch is skipped
if(pCtrl->socketClient != tempSocket && pCtrl->socketClient != NULL)
{
	[Close socketClient] 
}
  1. The tempSocket is assigned to the clientSocket and the initialization is finalized.
  2. After some time may a new client is accepted → now it is checked if the pointer of the tempSocket is actually the same as the socketClient:
  • If it is equal then the new client uses the same memory space as the old socket thus it must not be closed (it happens sometimes that the socket is closed by the stack apparently)
  • If the pointer is different go ahead and close the old client’s socket (here the socket is actively closed)

I think this implementation should not leak any memory or did I overlook something?

My platform is a Zynq7, and yes the error is repetitive. On every re-connectio I lose 64 Bytes of heap memory. As I have plenty of memory I actually have never run out of it up to now but I expect this to happen if the system runs long enough.

5

Hello Hein,
thanks for your reply.

Could you verify with your server if there is a memory loss on your system?

Thanks for your example altough for now I would prefer to keep using my current implementation. I’m explaining why I’ve chosen my approach in the reply post to @kanherea if you’re interrested.

“Make sure that after calling closesocket(), the socket is not referred to any more. It is a good habit to clear the pointer”
This is actually a very good suggestion, I will integrate it into my code, thank you!

How am I supposed to kick a client from server side? I just want to allow one client at a time to be connected, and always the latest that attempts to connect…

Is it advisable to use shutdown on server side at all then? Or is it also “common practice” to just close the socket?
Which would make the code easier anyways instead of adding a timeout like you suggested in your referenced post.

My code follows this rule, altough it may does not seem to. I just added the option to close the listening socket as this whole implementation is part of a port library that has the option to close the port on request.

Have you an idea why I could be loosing 64 Bytes? Do you have in mind some data structure of this size that gets re-allocated on every new connection?

Another thing that puzzles me is why the shutdown sometimes leaves the socket open and sometimes closes it. Is this related to the fact that the shutdown handshake needs time and so sometimes it manages to finish it and subsequently closes the socket as well, and sometimes it does not?

6

Hi Emanuel,

this is documented in depth in RFC733.

If I were you, the first thing I would do is keep global counters for how many times the accept() succeeds and how many times the corresponding closesocket() is indeed called. The way you code your control flow allows several inherent possibilities for leaks, as even your comments themselves imply.

Generally, there is no problem with your architecture in itself; both the one-thread-per-client and the sequential-accept-and-process-each-client-in-turn approach have their legitimate uses. But your control flow is fairly error prone, there is a lot to optimize, simplify and add robustness.

7

@Stonebull wrote:

Could you verify with your server if there is a memory loss on your system?

I did verify that, and I did not notice any loss of memory. I connected and disconnected telnet several times.

Thanks for your example altough for now I would prefer to keep using my current implementation. I’m explaining why I’ve chosen my approach in the reply post to @kanherea if you’re interrested.

Yet, I would recommend to study well the telnet source well, it explains a lot.

At first I implemented a server that allows only one client and re-uses the listener socket

That indeed will not work. You will have to call listen with a value of 2 or more for xBacklog.

I wrote:

Normally, a TCP server will not shut down a connection. It is the client who initiates a graceful shutdown.

How am I supposed to kick a client from server side?

In that case you can start a shutdown as I have described in my post on the FreeRTOS forum, summarised as shutdown, wait and try to read from socket, close socket.

I just want to allow one client at a time to be connected, and always the latest that attempts to connect…

All right, in that case, you will have to follow the procedure shutdown/wait/closesocket, to kill the older connection.

Is it advisable to use shutdown on server side at all then? Or is it also “common practice” to just close the socket?

As long as a connection is alive, it is considered “not done” to just close a socket. Doing so will lead to a TCP RST packet, which is seen as a kind of error condition.

Which would make the code easier anyways instead of adding a timeout like you suggested in your referenced post.

It is what it is :-), you will need this time-out for a gracious shut-down.

Have you an idea why I could be loosing 64 Bytes? Do you have in mind some data structure of this size that gets re-allocated on every new connection?

As far as I know, there are no memory leaks in the FreeRTOS kernel or in any of the FreeRTOS libraries.
Sometimes, when I must find a memory leak, I put some short logging in pvPortMalloc() and vPortFree(), by adding a parameter that shows the origin in the source.

64 bytes is probably the minimum allocation size, so you’re allocating anything between 1 and 64 bytes.

Another thing that puzzles me is why the shutdown sometimes leaves the socket open and sometimes closes it. Is this related to the fact that the shutdown handshake needs time and so sometimes it manages to finish it and subsequently closes the socket as well, and sometimes it does not?

The function FreeRTOS_shutdown() only sets a flag. The next message to the peer will contain the FIN flag, unless there is pending data traffic. So yes, every shutdown needs time to complete, up to a few seconds ( on the Internet ).

8

@htibosch

Thanks for your advice, in order to terminate a connection I now initiate a shutdown, wait for the connection to be down and the close the socket. I implemeted it a little differently, though it seems to works flawlessly as far as I can tell.

//Initiate a graceful shutdown
FreeRTOS_shutdown( pCtrl->socketClient, FREERTOS_SHUT_RDWR );

//Wait for the shutdown handshake to complete
do
{
	retVal_os = FreeRTOS_issocketconnected(pCtrl->socketClient);

	if(retVal_os == pdTRUE)
	{
		vTaskDelay(2 / portTICK_RATE_MS);
	}
} while(retVal_os == pdTRUE);

//Close the socket to free up memory
FreeRTOS_closesocket( pCtrl->socketClient );
pCtrl->socketClient = NULL;

You were perfetcly right, I FOUND the issue in my own implementation. I always forgot to delete the socket set after a connection was shut down.

FreeRTOS_DeleteSocketSet(pCtrl->socketSet);

Now I don’t get any memory leaks at all!

@ALL Thanks for your time and effort for looking into this.