Thread not exiting

[steg90]

Hi,

I have an application which starts a thread ( QThread ), to stop the thread I have a push button in the main gui, when pressed I want it to stop this thread, I thought I could simply do :

m_pMyThread->quit();

m_pMyThread->quit();

To copy to clipboard, switch view to plain text mode 

But alas, the thread carries on running?

Any ideas?

Regards,
Steve

[marcel]

Did you run exec() in QThread::run()?
If not, this function has no effect, because it tells the thread's event loop to exit with code 0, but in this case the thread doesn't have an event loop.

Could we see your run method?
You might wanna add a mCancelled member in your thread, and a function cancelThread(). Inititalliy mCancelled is false and is set true by cancelThread.

You call cancelThread instead of quit. Make sure to test mCancelled in the loops in your run method.

regards

[steg90]

Hi,

I don't call exec() from my run method, here it is :

void CanRead::run()
{
	m_nCount = 0;
 
	unsigned long nMsgAmount = 2;
 
	CANDATA can[MAX];
 
	QString szTemp;
 
	while( true )
	{
#ifdef __CANON_
		unsigned long numMsgs = 0;
		PASSTHRU_MSG *pRxMsg = NULL;
		pRxMsg = theApp->GetMessageDetail( &numMsgs, 0 );
		nMsgAmount = numMsgs;  // store amount of messages we got
		int index = 0;
		// Read can data, store in buffer ( read say N amount and then emit? )
		while ( numMsgs )
		{
			unsigned int iCanId = 0;
			for ( int iLoop = 0; iLoop < 4; iLoop++)
			{
				iCanId = (iCanId << 8 ) | pRxMsg->Data[iLoop];
			}
			if( pRxMsg->RxStatus & 0x100 )
				can[index].strId.sprintf( "%X X", iCanId);
			else
				can[index].strId.sprintf( "%03X", iCanId);
 
			for ( int iLoop = 4; (unsigned)iLoop < (pRxMsg->DataSize ); iLoop++)
			{
				szTemp.sprintf("%02X ", pRxMsg->Data[iLoop]);
				can[index].strData += szTemp;
			}
 
			can[index].strTime.sprintf("%04d", pRxMsg->Timestamp);
 
			numMsgs--;
 
			m_nCount++;
 
		}  // end while ( ulNoMsgs )
 
#else  // some dummy data for testing table view
 
		can[0].strId = "0xfea";
		can[0].strData = "222222222222";
		can[0].strTime = "----";
 
		can[1].strId = "0xfea";
		can[1].strData = "111111111111";
		can[1].strTime = "----";
 
		m_nCount += nMsgAmount;
 
#endif		
		// send out signal
		if( nMsgAmount > 0 )
			emit scrolltable( m_nCount - 2, nMsgAmount, &can[0] );
		msleep(1);
 
	} // end while ( true )
 
}

void CanRead::run()
{
	m_nCount = 0;
	
	unsigned long nMsgAmount = 2;

	CANDATA can[MAX];
	
	QString szTemp;
	
	while( true )
	{
#ifdef __CANON_
		unsigned long numMsgs = 0;
		PASSTHRU_MSG *pRxMsg = NULL;
		pRxMsg = theApp->GetMessageDetail( &numMsgs, 0 );
		nMsgAmount = numMsgs;  // store amount of messages we got
		int index = 0;
		// Read can data, store in buffer ( read say N amount and then emit? )
		while ( numMsgs )
		{
			unsigned int iCanId = 0;
			for ( int iLoop = 0; iLoop < 4; iLoop++)
			{
				iCanId = (iCanId << 8 ) | pRxMsg->Data[iLoop];
			}
			if( pRxMsg->RxStatus & 0x100 )
				can[index].strId.sprintf( "%X X", iCanId);
			else
				can[index].strId.sprintf( "%03X", iCanId);
		
			for ( int iLoop = 4; (unsigned)iLoop < (pRxMsg->DataSize ); iLoop++)
			{
				szTemp.sprintf("%02X ", pRxMsg->Data[iLoop]);
				can[index].strData += szTemp;
			}
			
			can[index].strTime.sprintf("%04d", pRxMsg->Timestamp);

			numMsgs--;

			m_nCount++;

		}  // end while ( ulNoMsgs )

#else  // some dummy data for testing table view
		
		can[0].strId = "0xfea";
		can[0].strData = "222222222222";
		can[0].strTime = "----";

		can[1].strId = "0xfea";
		can[1].strData = "111111111111";
		can[1].strTime = "----";

		m_nCount += nMsgAmount;

#endif		
		// send out signal
		if( nMsgAmount > 0 )
			emit scrolltable( m_nCount - 2, nMsgAmount, &can[0] );
		msleep(1);
	
	} // end while ( true )

}

To copy to clipboard, switch view to plain text mode 

Regards,
Steve

[marcel]

Yes, I assumed you did so.
Add the mCancelled member and replace the two "while" conditions with:

1. The first while:

while( !mCanceled )

while( !mCanceled )

To copy to clipboard, switch view to plain text mode 

2. The second while:

while( numMsgs && !mCancelled )

while( numMsgs && !mCancelled )

To copy to clipboard, switch view to plain text mode 

Declare mCancelled in the thread class as ( you can make it private ):

volatile bool mCancelled;

volatile bool mCancelled;

To copy to clipboard, switch view to plain text mode 

Also, add a new member function( public ):

void cancelThread();
...
void CanRead::cancelThread()
{
   mCancelled = true;
}

void cancelThread();
...
void CanRead::cancelThread()
{
   mCancelled = true;
}

To copy to clipboard, switch view to plain text mode 

All you have to do next is to call cancelThread instead of quit().
Note that it may take a little for the thread to stop, because it may be inside one of the loops when you set mCanceled to true. But this delay is acceptable, in my opinion, and you may not even notice it.

EDIT - Don;t forget to make mCancelled false in the constructor of the thread

Regards

[steg90]

Thanks Marcel, you are a great help.

Kind regards,
Steve