?
Solved

memory error when CTypedPtrArray<CPtrArray, ClassType*> element class has any virtual method

Posted on 2009-04-22
4
Medium Priority
?
1,091 Views
Last Modified: 2013-12-14
In VC++ 6.0 console application with linking MFC dll on windows 2000 server, if the element class of CTypedPtrArray has any virtual method(s), memory access error will occur under the following case: allocate a object of ClassType from heap with "new", add the pointer to this object onto CTypedPtrArray object, take and remove an element from the head of CTypedPtrArray, do sth (e.g. access its data) with the element, finally memory access error occurs while deleting the element.

The example  always has such a  memory access error regardless of relase/debug modes as long as the element class has any virtual methods. If we remove specifier 'virutal' before name of class methods, everything works fine in both debug and release modes.


struct PacketHeader
{
	unsigned long tag;		
	unsigned short msgid;	
	unsigned short status;	
	unsigned long timestamp;	
	char callerid[12];		
};
 
class CTaskObj
{
public:
	SOCKADDR_IN		cliAddr;
	// struct sockaddr_in	cliAddr;
	struct PacketHeader	msgHeader;
	char* pDataBuf;
	int		nBufSize;
 
public:
	CTaskObj();
//	 ~CTaskObj();
	virtual ~CTaskObj();
};
 
CTaskObj::CTaskObj()
{
	memset(&cliAddr, 0, sizeof(cliAddr));
	memset(&msgHeader, 0, sizeof(msgHeader));
 
	pDataBuf = NULL;
	nBufSize = 0;
}
 
CTaskObj::~CTaskObj()
{
	if (pDataBuf)
	{
		delete[] pDataBuf; pDataBuf = NULL;
	}
}
 
bool AddTaskIntoQueue(CTypedPtrArray<CPtrArray, CTaskObj*>& arrTasks, USHORT nUdpPort, USHORT nMsgID)
{
	CTaskObj* pTask = NULL;					
	try
	{
		SOCKADDR_IN addrClient;
		memset(&addrClient, 0, sizeof(addrClient));
		addrClient.sin_port = htons(nUdpPort);
 
		char pRecvBuf[1024];
		memset(pRecvBuf, 0, sizeof(pRecvBuf));
		PacketHeader* pAppHeader = (PacketHeader *)pRecvBuf;
		pAppHeader->msgid = htons(nMsgID);
 
		pTask = new CTaskObj;
		if (!pTask)
		{
			throw(-1);
		}
		memset(pTask, 0, sizeof(CTaskObj));
 
		// - copy received packet into task object
		memmove(&(pTask->cliAddr), &addrClient, sizeof(addrClient));
		memmove(&(pTask->msgHeader), pAppHeader, sizeof(PacketHeader));
 
		// no extra (optional) data are used for this task.
		pTask->pDataBuf = NULL;
 
		// - add the task into the task queue of UdpOutThread
		arrTasks.Add(pTask);
		pTask = NULL;	
	}
	catch(...)
	{
		printf("-- failed to add task into job queue. \n");
	}
 
	if (pTask)
	{
		delete pTask; pTask = NULL;
	}
 
	return true;
}
 
 
void TestTaskObjQueue()
{
	CTypedPtrArray<CPtrArray, CTaskObj*> arrTasks;
	arrTasks.SetSize(0, 10);
 
	USHORT nUdpPort = 100;
	USHORT nMsgID = 1000;
 
	AddTaskIntoQueue(arrTasks, nUdpPort, nMsgID);
	printf("--- num of items in queue = %d \n", arrTasks.GetSize());
 
	CTaskObj* pTask = arrTasks.GetAt(0);
	arrTasks.RemoveAt(0);
 
	int port = ntohs(pTask->cliAddr.sin_port);
	int mid = ntohs(pTask->msgHeader.msgid);
	printf("---data: port=%u, msgid=%u ; num of items=%d \n", port, mid, arrTasks.GetSize());
 
	if (pTask)
	{
		delete pTask; pTask = NULL;
	}
}
 
int main(int argc, char** argv)
{
 TestTaskObjQueue();
return 0;
}

Open in new window

0
Comment
Question by:xing2kin
  • 2
  • 2
4 Comments
 
LVL 31

Accepted Solution

by:
Zoppo earned 1000 total points
ID: 24203620
Hi xing2kin,

I think you should remove this code (line 61):
> memset(pTask, 0, sizeof(CTaskObj));

This IMO overwrites the pointer to the class's VTABLE (that's where the pointers to the virtual functions are stored). Since your class CTaskObj has a constructor it's no needed.

Attached you can find an example which shows how such classes differ with/without virtual functions - it's output is something like this:

> sizeof( T1 ): 4 - addr of 't1': 0024FC64 - addr of t1.x: 0024FC64
> sizeof( T2 ): 8 - addr of 't2': 0024FC54 - addr of t2.x: 0024FC58

So, without a VTABLE the pointer to the class-instance is the same as the pointer to the first member. With a VTABLE the pointer to the class instance is a pointer to the VTABLE which is followed by the first member.

Hope that helps,

ZOPPO

class T1
{
public:
 int x;
 ~T1() {}
};
 
class T2
{
public:
 int x;
 virtual ~T2() {}
};
 
int main( int argc, char* argv[] )
{
 T1 t1;
 T2 t2;
 
 std::cout << "sizeof( T1 ): " << sizeof( T1 ) << " - addr of 't1': " << std::hex << std::setw( 8 ) << std::setfill( '0' ) << &t1 << " - addr of t1.x: " << &t1.x << std::endl;
 std::cout << "sizeof( T2 ): " << sizeof( T2 ) << " - addr of 't2': " << std::hex << std::setw( 8 ) << std::setfill( '0' ) << &t2 << " - addr of t2.x: " << &t2.x << std::endl;
 
 return 0;
}

Open in new window

0
 

Author Comment

by:xing2kin
ID: 24205406
Yeah, your solution helps fix the problem.  Thanks a lot!!!!

After commenting out the line [memset(pTask, 0, sizeof(CTaskObj))],  memory error because of [delete pTask] never occurs.

According to your comments on the above, conclusion might be drawn that one should NEVER intialize the dynamically-allocated  memory buffer of a class object with 'memset(-)' if the class has any virtual method(s) (i.e. it has VTABLE). Since  'memset(-)'  resets class' VTABLE, deleting the object buffer will
cause 'memory access error'.
0
 

Author Closing Comment

by:xing2kin
ID: 31573199
You provided a good solution to this probblem. Furthermore, your comment helps understand why this problem occurs.
0
 
LVL 31

Expert Comment

by:Zoppo
ID: 24205542
Hi xing2kin,

you're welcome - fine that I was able to help ...

And yes, I agree, one should never do this - in any case it's better to implement a suitable constructor, this help avoiding errors and minimizes the effort (because no one has to remember the object needs to be 'zero-ed' after creation). Further one can never say if maybe in future some members will be added which shouldn't ever be set to zero since they are initialized to something differen in the constructor.

Have a nice day,

best regards,

ZOPPO
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Introduction: Dynamic window placements and drawing on a form, simple usage of windows registry as a storage place for information. Continuing from the first article about sudoku.  There we have designed the application and put a lot of user int…
Jaspersoft Studio is a plugin for Eclipse that lets you create reports from a datasource.  In this article, we'll go over creating a report from a default template and setting up a datasource that connects to your database.
The viewer will learn how to use NetBeans IDE 8.0 for Windows to connect to a MySQL database. Open Services Panel: Create a new connection using New Connection Wizard: Create a test database called eetutorial: Create a new test tabel called ee…
THe viewer will learn how to use NetBeans IDE 8.0 for Windows to perform CRUD operations on a MySql database.
Suggested Courses

616 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question