Serial COM + Multi threading gives heap corruption error

Question

0.00/5 (No votes)

See more:

Dear fellow devs,

I am having some problem with the heap. Basically I spin up 2 threads one for receiving and one for sending via the serial com port. I declare a new pointer to a struct MsgBlk I defined in the header. the program keeps crashing at the delete MsgBlk when i activate the sendthread. When i remove the delete statement, and when the sendthread is activated, the program crashes at the new MsgBlk delcaration. Read up and it seems like a race condition or double deletion, but i have not used this msgblk anywhere else except here.

the crash only happens when i try to activate the send thread. can anyone tell me whats happening??

C++

unsigned long ReceiveThread(CMyTabs* myTab)
{
    while(true)
    {
    	if(!initialisePort)
	{
		//LONG error = serial.Open(_T("\\\\.\\\\COM5"));

		// Setup the serial port (9600,N81) 
		initialisePort = true;
	}
		
	DWORD dwBytesRead = 0;
	byte abBuffer[256];
	bool crcCheck = false;
	bool msgCheck = false;
	// declare msgBlk
	MsgBlk *msgBlk = new MsgBlk();
	do
	{
		crcCheck = msgBlk->fValidCRC;
		msgCheck = msgBlk->fValid;
		// do polling for data
		serial.Read(abBuffer, 256, &dwBytesRead, 0, INFINITE);
		// if bytes more than 0
		if(dwBytesRead > 0)
		{
			myTab->stripMsg(abBuffer, sizeof(abBuffer), msgBlk);
		}
		Sleep(50);
	}while(!msgCheck && !crcCheck);
		
	char szTimestamp[128];
	SYSTEMTIME st;
	//GetSystemTime(&st);
	GetLocalTime(&st);
	sprintf_s(szTimestamp, 128, "%02d:%02d:%02d:%03d", st.wHour, st.wMinute, st.wSecond, st.wMilliseconds);

	byte bytetobesent[256];
	for(int k=0; k<256; ++k)
	{
		bytetobesent[k] = msgBlk->msg[k];
	}

	PostMessage(myTab->GetSafeHwnd(), WM_MSG_RECV, (WPARAM)bytetobesent, (LPARAM)szTimestamp);
	delete msgBlk;
}
return 0;
}


unsigned long SendThread(CMyTabs* myTab)
{
	while(true)
	{
		while(!sendMsgFlag)
		{
			Sleep(0);
		}
		++sendCounter;
		sendMsgFlag = false;
		// send message 41
		DWORD dwBytesWritten=0;
		int in_len;
		int	out_len;
		int oplen;
		byte inbuf[5];
		byte outbuf[512];

		inbuf[0] = 0x06;	
		inbuf[1] = 0x33;	
		inbuf[2] = sendCounter;	
		inbuf[3] = (sendCounter >> 8);	
		inbuf[4] = 112;		

		out_len = in_len = 5;

		oplen = myTab->packMsg(inbuf, in_len, outbuf, out_len);
		
		long Lerror = serial.Write(outbuf, oplen);
		
		if(Lerror != ERROR_SUCCESS)
		{
			AfxMessageBox(_T("Unable to write to COM Port!"));
		}
	}
	return 0;
}

Posted 30-Oct-11 18:01pm

Zsefv2

Updated 31-Oct-11 12:38pm

v7

Add a Solution

Comments

Mehdi Gholam 31-Oct-11 0:49am

Using a serial port is in the name, it should be done sequentially and in one thread, try sending and recieving in one thread.

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Andrew Brock · Answer 1 · 2011-10-30T20:29:00

You may not have thread-safe code in serial.Read or serial.Write
stripMsg and packMsg may also have issues.

I don't see any issues which would be causing crashes here, but that doesn't mean there are some other bad things.

You should use events[^] instead of CPU hungry loops (at the top of SendThread).

C++

unsigned long ReceiveThread(CMyTabs* myTab)
{
    while(true)
    {
    	if(!initialisePort)
	{
		//LONG error = serial.Open(_T("\\\\.\\\\COM5"));

		// Setup the serial port (9600,N81) 
		initialisePort = true;
	}
		
	DWORD dwBytesRead = 0;
	byte abBuffer[256];
	bool crcCheck = false;
	bool msgCheck = false;
	// declare msgBlk
	MsgBlk *msgBlk = new MsgBlk();
	do
	{
		crcCheck = msgBlk->fValidCRC;
		msgCheck = msgBlk->fValid;
		// do polling for data
		serial.Read(abBuffer, 256, &dwBytesRead, 0, INFINITE);
		// if bytes more than 0
		if(dwBytesRead > 0)
		{
			myTab->stripMsg(abBuffer, sizeof(abBuffer), msgBlk);
		}
		Sleep(50); //Sleep is typically bad style. You can set time-outs for reading files.
	}while(!msgCheck && !crcCheck);
		
	char szTimestamp[128];
	SYSTEMTIME st;
	//GetSystemTime(&st);
	GetLocalTime(&st);
	sprintf_s(szTimestamp, 128, "%02d:%02d:%02d:%03d", st.wHour, st.wMinute, st.wSecond, st.wMilliseconds);
 
	byte bytetobesent[256];
	//What is wrong with memcpy?
	/*for(int k=0; k<256; ++k)
	{
		bytetobesent[k] = msgBlk->msg[k];
	}*/
	memcpy(bytetobesent, msgBlk->msg, sizeof(bytetobesent));
	//This is bad. What happens to bytetobesent here?
	//It is destroyed when the function exits, which is possibly before the handler for WM_MSG_RECV is invoked.
	//You will need to create a global buffer or allocate memory on the heap with malloc/new
	//Alternativley you can use SendMessage, which will invoke and complete WM_MSG_RECV before returning back here.
	PostMessage(myTab->GetSafeHwnd(), WM_MSG_RECV, (WPARAM)bytetobesent, (LPARAM)szTimestamp);
	delete msgBlk;
    }
    return 0;
}
 

unsigned long SendThread(CMyTabs* myTab)
{
	while(true)
	{
		//This is really bad. Use an Event. See notes above.
		/*while(!sendMsgFlag)
		{
			Sleep(0);
		}*/
		WaitForSingleObject(hMyEvent, INFINITE);
		++sendCounter;
		sendMsgFlag = false;
		// send message 41
		DWORD dwBytesWritten=0;
		int in_len;
		int	out_len;
		int oplen;
		byte inbuf[5];
		byte outbuf[512];
 
		//AfxMessageBox(_T("Testing WIA req"));
		inbuf[0] = 0x06;	
		inbuf[1] = 0x33;	
		inbuf[2] = sendCounter;	
		inbuf[3] = (sendCounter >> 8);	
		inbuf[4] = 112;		
 
		out_len = in_len = 5;
 
		oplen = myTab->packMsg(inbuf, in_len, outbuf, out_len);
		
		long Lerror = serial.Write(outbuf, oplen);
		
		if(Lerror != ERROR_SUCCESS)
		{
			AfxMessageBox(_T("Unable to write to COM Port!"));
		}
	}
	return 0;
}

Chuck O'Toole · Answer 2 · 2011-10-31T16:39:00

FYI - heap corruption problems are reported when you request heap functions and not necessarily when the corruption occurs. Your "new" or "delete" functions activate the heap checking and any corruption that occurred will then be reported. So, looking at "msgBlk" may be a "misdirection", the problem may really occur elsewhere, maybe even in another thread.

Both your threads use functions in a "myTab" object, it might help to post them as has been suggested elsewhere.

Serial COM + Multi threading gives heap corruption error

2 solutions

Solution 1

Solution 2

Add your solution here

Preview 0