Clean up memory at socket closure

SNMP++/AGENT++
1

Hello,

I have an application with agent++/snmp++. After a few hours running the application it crashes. I have run it with valgrind to check if I had a memory leak and it outputs:

==30203== Memcheck, a memory error detector
==30203== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30203== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==30203== Command: /tmp/MySNMPa
==30203== Parent PID: 25484
==30203==
==30203== Thread 17:
==30203== Invalid read of size 8
==30203==    at 0x565C740: get (List.h:490)
==30203==    by 0x565C740: Agentpp::RequestList::get_request(unsigned long) (request.cpp:823)
==30203==    by 0x19F9AD: MyMibManager::start(MyMibManager*) (in /tmp/MySNMPa)
==30203==    by 0x50AFE3E: thread_proxy (in /lib/libMessageDealerLib.so)
==30203==    by 0x4E3F493: start_thread (pthread_create.c:333)
==30203==    by 0x64CFAFE: clone (clone.S:97)
==30203==  Address 0xeaa1960 is 16 bytes inside a block of size 24 free'd
==30203==    at 0x4C2D2DB: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30203==    by 0x5664D8A: remove (List.h:309)
==30203==    by 0x5664D8A: Agentpp::List<Agentpp::Request>::remove(Agentpp::Request*) (List.h:273)
==30203==    by 0x5660677: Agentpp::RequestList::answer(Agentpp::Request*) (request.cpp:1169)
==30203==    by 0x1AD605: SnmpReqList::answer(Agentpp::Request*) (in /tmp/MySNMPa)
==30203==    by 0x5639F55: Agentpp::Mib::finalize(Agentpp::Request*) (mib.cpp:4015)
==30203==    by 0x563B4FF: Agentpp::Mib::do_process_request(Agentpp::Request*) (mib.cpp:3499)
==30203==    by 0x567DC0F: Agentpp::TaskManager::run() (threads.cpp:996)
==30203==    by 0x567EFD2: Agentpp::thread_starter(void*) (threads.cpp:705)
==30203==    by 0x4E3F493: start_thread (pthread_create.c:333)
==30203==    by 0x64CFAFE: clone (clone.S:97)
==30203==  Block was alloc'd at
==30203==    at 0x4C2C21F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30203==    by 0x565CFA5: addLast (List.h:99)
==30203==    by 0x565CFA5: add (List.h:113)
==30203==    by 0x565CFA5: Agentpp::RequestList::add_request(Agentpp::Request*) (request.cpp:1645)
==30203==    by 0x5663218: Agentpp::RequestList::receive(int) (request.cpp:1578)
==30203==    by 0x19DDEA: MyMibManager::start(MyMibManager*) (in /tmp/MySNMPa)
==30203==    by 0x50AFE3E: thread_proxy (in /lib/libMessageDealerLib.so)
==30203==    by 0x4E3F493: start_thread (pthread_create.c:333)
==30203==    by 0x64CFAFE: clone (clone.S:97)
==30203==
==30203== Invalid read of size 8
==30203==    at 0x565C752: next (List.h:496)
==30203==    by 0x565C752: Agentpp::RequestList::get_request(unsigned long) (request.cpp:823)
==30203==    by 0x19F9AD: MyMibManager::start(MyMibManager*) (in /tmp/MySNMPa)
==30203==    by 0x50AFE3E: thread_proxy (in /lib/libMessageDealerLib.so)
==30203==    by 0x4E3F493: start_thread (pthread_create.c:333)
==30203==    by 0x64CFAFE: clone (clone.S:97)
==30203==  Address 0xeaa1958 is 8 bytes inside a block of size 24 free'd
==30203==    at 0x4C2D2DB: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30203==    by 0x5664D8A: remove (List.h:309)
==30203==    by 0x5664D8A: Agentpp::List<Agentpp::Request>::remove(Agentpp::Request*) (List.h:273)
==30203==    by 0x5660677: Agentpp::RequestList::answer(Agentpp::Request*) (request.cpp:1169)
==30203==    by 0x1AD605: SnmpReqList::answer(Agentpp::Request*) (in /tmp/MySNMPa)
==30203==    by 0x5639F55: Agentpp::Mib::finalize(Agentpp::Request*) (mib.cpp:4015)
==30203==    by 0x563B4FF: Agentpp::Mib::do_process_request(Agentpp::Request*) (mib.cpp:3499)
==30203==    by 0x567DC0F: Agentpp::TaskManager::run() (threads.cpp:996)
==30203==    by 0x567EFD2: Agentpp::thread_starter(void*) (threads.cpp:705)
==30203==    by 0x4E3F493: start_thread (pthread_create.c:333)
==30203==    by 0x64CFAFE: clone (clone.S:97)
==30203==  Block was alloc'd at
==30203==    at 0x4C2C21F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30203==    by 0x565CFA5: addLast (List.h:99)
==30203==    by 0x565CFA5: add (List.h:113)
==30203==    by 0x565CFA5: Agentpp::RequestList::add_request(Agentpp::Request*) (request.cpp:1645)
==30203==    by 0x5663218: Agentpp::RequestList::receive(int) (request.cpp:1578)
==30203==    by 0x19DDEA: MyMibManager::start(MyMibManager*) (in /tmp/MySNMPa)
==30203==    by 0x50AFE3E: thread_proxy (in /lib/libMessageDealerLib.so)
==30203==    by 0x4E3F493: start_thread (pthread_create.c:333)
==30203==    by 0x64CFAFE: clone (clone.S:97)

After this message appears the application crashes.
As it seems that MyMibManager::start causes the crash, my programming routine to process requests is:

// MyMibManager.h class MyMibManager : public NotificationOriginator { ... /* Port of SNMP communications */ unsigned short port; /* Snmpx object to manage socket subsytem */ Agentpp::Snmpx *snmp; /* Mib Object */ Agentpp::Mib *mib; /* Non ordered queue of Requests */ SnmpReqList *reqList; /* Indicates if the SNMP server is running or not */ bool isStopped; ... } 
class SnmpReqList: public Agentpp::RequestList
{
	/* Print message if it is SETREQUESTPDU */
	virtual void answer(Agentpp::Request* req);
}

// MyMibManager.cpp
#define NOT_RESPONDING_LIMIT_US		5000
#define NOT_RESPONDING_LIMIT_CNT	10
void MyMibManager::start(MyMibManager* pMyMibMng)
{
	Agentpp::Request * req = NULL;
	int subReqNum;

	unsigned long cnt = 0;

	// Loop until externally controlled
	while(!pMyMibMng->isStopped){
		req = pMyMibMng->reqList->receive(1); //Wait for a given time for an incoming request.
		if (req){
			subReqNum = req->subrequests();
			if (subReqNum > 0){
				// Process the request... if available thread in pool
				cnt = 0;
				do{
					if (pMyMibMng->mib->get_thread_pool()->is_busy() == true){
						cnt ++;
						usleep(NOT_RESPONDING_LIMIT_US);
					}
					else{
						pMyMibMng->mib->process_request(req);		//Process the request.
						break;
					}
				}while (cnt < NOT_RESPONDING_LIMIT_CNT);
				
				// In this case the request has not been processed
				if (cnt >= NOT_RESPONDING_LIMIT_CNT){
					printRequestToFile(req);

					// Unable to process this request. It can be removed from the request list and deleted
					if (req){
						pMyMibMng->reqList->remove_request(req);
					}

					// Print all pending/problematic requests to file
					for (unsigned int j = 0; j < pMyMibMng->reqList->size(); j++){
						auxreq = pMyMibMng->reqList->get_request(j);
						if (auxreq){
							printRequestToFile(req);
						}
					}
				}
			}
			else{
				// Received request with no sub-requests, discard it!
				pMyMibMng->reqList->remove_request(req);
			}
		}
		else{
			pMyMibMng->mib->cleanup(); //Clean up MIB (remove unused table rows)
		}
	}
}

I noticed that sometimes my application processes too much requests and there are no more available threaads to process new requests in the mib (Agentpp::Mib typed object). To prevent myself from that case I decided to check if I could process a request a few times and discard all requests that would not be processed.

Another query I have is:
Could my Agentpp::RequestList affect to performance? I overwrite that function to send a message to my server and log the request in a file.

Any help will be appreciated.

Thanks in advance!

2

The design error in the MyMibManager::start method is located in the following code

				// Print all pending/problematic requests to file
				for (unsigned int j = 0; j < pMyMibMng->reqList->size(); j++){
					auxreq = pMyMibMng->reqList->get_request(j);
					if (auxreq){
						printRequestToFile(req);
					}
				}

The above code iterates on the reqList while other threads are removing entries in that list. To make this code save, you have to lock the whole reqList before and after this code block using:

{ ThreadSynchronize _ts_synchronize(*pMyMibMng->reqList); 
	// Print all pending/problematic requests to file
	for (unsigned int j = 0; j < pMyMibMng->reqList->size(); j++){
		auxreq = pMyMibMng->reqList->get_request(j);
		if (auxreq){
			printRequestToFile(req);
		}
	}
}

Overall performance is influenced mainly by the instrumentation code and table lookups. Overwriting RequestList should not have any negative effect if the additional code performs well.

1 Like