[SOLVED] Thread class in c++ isn't releasing stack

[fredheeks]

I have written a C++ thread class to process a socket connection.
Basically, the main process listens for incoming connection requests and then when it receives one it accepts the connection and creates a new thread class to process it.

This thread class (note - CLASS) takes the new socket handle and creates a pthread to process the incoming data. It then waits (pthread_join) for the thread to finish processing and exits.

The problem I have is that the thread stacks are not being released.
The class creates the new thread and a stack is allocated (which I can see in /proc/<PID>/smaps.
The thread class does wait for the thread to finish and then exits, but the thread stack is still left there. There must be some problem releasing the thread resources somewhere but I just cannot see it. Valgrind doesn't report any memory leaks either!

Some of the code is attached to see if you can spot anything amiss:

Main code loop:

// gethostbyname() takes a host name or ip address in "numbers and
	// dots" notation and returns a pointer to a hostent structur.
	hostInfo = gethostbyname(CConfig::host.c_str());
	if (hostInfo == NULL) {
		CLogger::m_pInstance->logF("Problem interpreting host " + CConfig::host);
		return result;
	}

	// Create the socket and listen for connections
	listenSocket = socket(AF_INET, SOCK_STREAM, 0);
	if (listenSocket < 0) {
		CLogger::m_pInstance->logF("Cannot create listen socket");
		return result;
	}

	// Bind listen socket to listen port.  First set various fields in
	// the serverAddress structure, then call bind().
	// htonl() and htons() convert long integers and short integers
	// (respectively) from host byte order (on x86 this is Least
	// Significant Byte first) to network byte order (Most Significant
	// Byte first).
	serverAddress.sin_family = AF_INET;
	serverAddress.sin_addr.s_addr = htonl(INADDR_ANY);
	serverAddress.sin_port = htons(CConfig::port);

	if (bind(listenSocket,(struct sockaddr *) &serverAddress,sizeof(serverAddress)) < 0) {
		CLogger::m_pInstance->logF("Cannot bind socket");
		return result;
	}
	
	// Wait for connections from clients.
	// This is a non-blocking call; i.e., it registers this program with
	// the system as expecting connections on this socket, and then
	// this thread of execution continues on.
	if (listen(listenSocket, CConfig::maxSockets) < 0) {
		CLogger::m_pInstance->logF("Cannot listen on address " + CConfig::host + " and port " + CStringHelper::intToString(CConfig::port));
		CLogger::m_pInstance->logF("Check that the address and port is not already in use");
		return result;
	}

	CLogger::m_pInstance->logF("-------< Beginning listen loop " +CStringHelper::getDateTime() + " >-------");

	while (listenSocket) {
		int waitCount = 0;
		int msgsock;
		int checkCount = 0;
		int rcvCount = 0;
		bool setTimeOut = false;
		struct timeval tv;
		long timedOut = 0;

		clientAddressLength = sizeof(clientAddress);
		// First set line to all zeroes, so we'll know where
		// the end of the string is.
		memset(line, 0x00, LINE_ARRAY_SIZE);
		msgsock = accept(listenSocket, (struct sockaddr*)&clientAddress, &clientAddressLength);
		if (msgsock < 0) {
			CLogger::m_pInstance->logF("Cannot accept connection - the service will exit");
			CLogger::m_pInstance->logF("Use 'free -m' to see if the server has sufficient free memory");
			continue;
		} else {
			// show the client's IP address
			string adr = inet_ntoa(clientAddress.sin_addr);
			string port = CStringHelper::intToString(ntohs(clientAddress.sin_port));
			CLogger::m_pInstance->logF("Accepted connection from "+adr+":"+port);

			// Check if there are any spare threads
			if (CThread::numberOfThreads < CConfig::maxThreads) {
				CThread *con = new CThread(&cbFunction, port, msgsock);

				if (con->ret == 0) {
					CLogger::m_pInstance->logD("ThreadCount: "+CStringHelper::intToString(CThread::numberOfThreads));
				} else {
					shutdown(msgsock,SHUT_RDWR);
					close(msgsock);
				}
			} else {
				CLogger::m_pInstance->logF("There are no spare threads available. Unable to start processing this job");
				shutdown(msgsock,SHUT_RDWR);
				close(msgsock);
			}

			// Wait to check again
			sleep(CConfig::waitDelay);
		} // if (msgsock < 0)	
	} // while listenSocket

Thread class:

#include "thread.h"

// Initialise static variables
int CThread::numberOfThreads = 0;

/*
 * Constructor
 */
CThread::CThread(void (*cb)(CThread* ct), string message, int sock) {
	m_pInstance = this;
	callback = cb;
	arg = message;
	msgsock = sock;
	int status = 0;

	// Increase the thread count. If the thread doesn't create successfully the
	// thread count will still get decreased by the destructor	
	numberOfThreads++;

	ret = pthread_create(&handle, NULL, CThread::entryPoint, this);
	pthread_detach(handle);
	if (ret != 0 ) {
		CLogger::m_pInstance->logF("Thread "+arg+": Problem creating processing thread");
		CLogger::m_pInstance->logF("Thread "+arg+": Return code is "+CStringHelper::intToString(ret));
		closeSocket();

		// Remove the class
		delete this;
	}

	CLogger::m_pInstance->logD("Thread "+arg+": Class constructor has returned");
}

/*
 * Destructor
 */
CThread::~CThread() {
	numberOfThreads--;
	CLogger::m_pInstance->logD("ThreadCount: "+CStringHelper::intToString(CThread::numberOfThreads));
}

/*static */
void * CThread::entryPoint(void * pthis)
{
	int ret=0;
	CThread* pt = (CThread*)pthis;
	pt->execute();
	// Wait for thread to finish
	pthread_join(pt->handle, NULL);
	CLogger::m_pInstance->logF("Thread "+pt->arg+": Finished processing. Thread will close");
	delete pt;
}

void * CThread::execute()
{
	// Detach thread to allow concurrent processing
	//pthread_detach(pthread_self());
	CLogger::m_pInstance->logD("Thread "+arg+": Thread is executing");

	int waitCount = 0;
	int checkCount = 0;
	int rcvCount = 0;
	bool setTimeOut = false;
	long timedOut = 0;
	char line[LINE_ARRAY_SIZE];
	bool processing = true;

	CLogger::m_pInstance->logF("Thread "+arg+": Processing incoming data");

	// First set line to all zeroes, so we'll know where
	// the end of the string is.
	memset(line, 0x00, LINE_ARRAY_SIZE);

	// Create file parser
	CDataParser *dataParser = new CDataParser(CConfig::archiveDir, arg);

	// Create temporary file
	bool connectionOpen = true;

	// Process the accepted connection
	//NOTE - will keep processing the socket until no data is received	while (processing) {
		// Process input stream
		while(connectionOpen && (checkCount = recv(msgsock, line, MAX_MSG, MSG_PEEK) > 0)) {
			rcvCount = recv(msgsock, line, MAX_MSG, 0);
			if (rcvCount < 0) {
				CLogger::m_pInstance->logF("Thread "+arg+": I/O Problem. Data was expected, but could not be retrieved.");
				closeSocket();
				processing = false;
				continue;
			}

			// We have received data so set the timeout check
			setTimeOut = true;

			// Parse the line
			connectionOpen = dataParser->parseLine(line, rcvCount);

			memset(line, 0x00, LINE_ARRAY_SIZE);  // set line to all zeroes

			// If the end was found then close the connection
			if (!connectionOpen) {
				closeSocket();
				processing = false;
			}
		} // while we're receiving data...

		// If there is no more data on the wire then check how long we have been
		// listening for. If it exceeds the timeout, release the connection
		if (checkCount == 0) {
			CLogger::m_pInstance->logF("Thread "+arg+": No data received");
			closeSocket();
			processing = false;
		}

		if (checkCount < 0) {
			CLogger::m_pInstance->logF("Thread "+arg+": Client closed socket connection.");
			closeSocket();
			processing = false;
		}
		// This gives the other threads time to do work.
		pthread_yield();
	} // while(msgsock)
	dataParser->closeFile();

	CLogger::m_pInstance->logF("Thread "+arg+": Connection closed");


	// Move or delete the data file, depending on the configuration
	if (CConfig::retainData) {
		// Keep temporary file if it's there
		if (dataParser->fileInDir(CConfig::archiveDir, dataParser->filename) != "") {
			CLogger::m_pInstance->logF("Thread "+arg+": Data has been successfully archived to \""+CConfig::archiveDir+dataParser->filename+"\"");
		}
	} else {
		remove((CConfig::archiveDir+dataParser->filename).c_str());
	}


	// Release the dataparser resources
	delete dataParser;
}

/*
 * Close this socket
 */
void CThread::closeSocket() {
	int status = 0;

	if (shutdown(msgsock,SHUT_RDWR) != 0) {
		switch(errno) {
			case EBADF:
				CLogger::m_pInstance->logF("Thread "+arg+": Problem closing this socket. Invalid socket descriptor.");
				break;
			case ENOTCONN:
				CLogger::m_pInstance->logF("Thread "+arg+": Socket closed.");
				break;
			case ENOTSOCK:
				CLogger::m_pInstance->logF("Thread "+arg+": Problem closing this socket. It does not appear to be a valid socket.");
				break;
			default:
				CLogger::m_pInstance->logF("Thread "+arg+": Problem closing this socket: "+CStringHelper::intToString(ret));
				break;
		}
	}
	status = close(msgsock);
	if (-1 == status) {
		CLogger::m_pInstance->logF("Thread "+arg+": Problem closing this socket: "+CStringHelper::intToString(errno));
	}
}

[Rubberman]

Please post code inside code blocks, as in:

// Example code - preserving indentations, etc
int myfunc(void)
{
    int retval = 0;
    return retval;
}

It will be a LOT easier to read and reply to.

[fredheeks]

OK - Thanks. I've changed that.

[fredheeks]

Right - I've gone back to basics and just written a really simple program to fire off a single thread to count with:

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

/*
 * Thread method
 */
void *threadMethod(void *arg)
{
	int lim = (int)arg;
	for (int i=1; i <= lim; i++)	// Count up forever
	{
		printf("%d\n",i);	// Output the current count
		sleep(1);		// Sleep for 1 second
	}

	pthread_exit(NULL);
}

/*
 * Main program entry point
 */
int main(int argc, char* argv[])
{
	pthread_t handle;	// Handle for the thread
	int ret = 0;		// Return code used for thread creation
	int limit = 0;

	if (argc == 2) {
		limit = atoi(argv[1]);
	}

	printf("Press enter to start counting\n");
	while (getchar() == 0) {}

	// Create the thread, storing the handle in 'handle', no custom attributes are used,
	// the method 'threadMethod' will be run as the thread and the count limit passed to it
	ret = pthread_create(&handle, NULL, threadMethod, (void*)limit);
	if (ret != 0 ) {
		printf("There was a problem creating the thread\n");
		printf("Return code is %d",ret);
		exit(EXIT_FAILURE);
	} 

	// Wait for the thread to finish
	pthread_join(handle, NULL);
	printf("\nThread finished\n");
	printf("Press enter to finish\n");

	// Wait for another 'enter' press to give time to check the memory map
	while (getchar() == 0) {}

	exit(EXIT_SUCCESS);
}

I'm compiling it with g++ -lpthread test.cpp -o test

It waits for you to press return and then counts up a number of times, then waits for you to press return before exiting. The waits are to give me a chance to check the memory map for the program.

When I run it, with ./test 10 it waits. so I do a ps -ef | grep test and make a note of the PID.
Then do a pmap <PID> to view the memory map and verify that there is no stack there:

<... snip ...>
b7f17000     12K rw---    [ anon ]
b7f38000      8K rw---    [ anon ]
bfb24000     84K rw---    [ stack ]

When I hit return to start the thread, it begins to count, so I check the memory map again:

<... snip ...>
09693000    132K rw---    [ anon ]
b7516000      4K -----    [ anon ]
b7517000  10252K rw---    [ anon ]
b7f38000      8K rw---    [ anon ]
bfb24000     84K rw---    [ stack ]

The user stack limit (ulimit -s) is set to 10240 and since there is always a 4k 'safety zone' created as well, that explains the chunks at 0xb7516000 and 0xb7517000 (although I am confused as to why the original 12k chunk at 0xb7f17000 has been extended by 10240, rather than a new chunk allocated).

The thread then finishes so before I press return to exit the program I check the memory map again:

<... snip ...>
09693000    132K rw---    [ anon ]
b7516000      4K -----    [ anon ]
b7517000  10252K rw---    [ anon ]
b7f38000      8K rw---    [ anon ]
bfb24000     84K rw---    [ stack ]

Now, the thread has finished, I've got the message to say so. The thread function is calling pthread_exit(NULL) and the main thread is waiting for it so WHY is the stack space still allocated?

Is there something else I have to do to de-allocate this?
Am I misunderstanding how pthreads works completely?
Is there someone out there that is familiar with pthreads to tell me where I am going wrong... PLEASE!

[fredheeks]

OK - I think that it is actually working.

I've done further tests with many, many concurrent threads (rather than just one or two) and it seems that for each thread a separate stack is created, plus the 4k 'red zone'; which is expected.

When the threads finish these stacks ARE reclaimed.

What has confused me is that there are always three stack memory blocks left there. This is why when testing with one or two threads it looked like the memory was not being freed.

If I run the tests again (on the same running process) then these three memory blocks are re-used.

It is a little strange; why there are always these extra blocks that are never freed totally, but as long as the program never grows beyond that it should be OK.

Of course, if anyone can come up with an explanation........ that would be appreciated.