QMultiHash - crashing

[steg90]

Hi,

I have the following code :

QList<QObject*>Signals;
 
	if( theApp->m_dcb.m_oSignalMap.count() > 0 )
	{
		Signals = theApp->m_dcb.m_oSignalMap.values( strId ) ;
		if( Signals.size() > 0 )
		{
			CDADcb::CSignal* pSignal = (CDADcb::CSignal*)Signals.at(0);
 
			// This signal belong to this CAN id, there can be many signals that do
			{
				// we need to do the conversion here - TODO
				pSignal->m_strRawData = strData;  
				pSignal->m_nCount++;
			}
		}
	}

QList<QObject*>Signals;

	if( theApp->m_dcb.m_oSignalMap.count() > 0 )
	{
		Signals = theApp->m_dcb.m_oSignalMap.values( strId ) ;
		if( Signals.size() > 0 )
		{
			CDADcb::CSignal* pSignal = (CDADcb::CSignal*)Signals.at(0);

			// This signal belong to this CAN id, there can be many signals that do
			{
				// we need to do the conversion here - TODO
				pSignal->m_strRawData = strData;  
				pSignal->m_nCount++;
			}
		}
	}

To copy to clipboard, switch view to plain text mode 

m_oSignalMap is a QMultiHash<QString, QObject*> of which CSignal objects are stored keyed on a string value. For some reason, this bit of code causes my application to crash at incoherent intervals, sometimes after a minute, sometimes after 5. The crash is in QHash.h when trying to find the key ( the key seems to be a bad ptr? ).

Sorry I can explain more, just seems so hit and miss...

Regards,
Steve

[freeskydiver]

QList<QObject*>Signals;
 
	if( theApp->m_dcb.m_oSignalMap.count() > 0 )
	{
		Signals = theApp->m_dcb.m_oSignalMap.values( strId ) ;

QList<QObject*>Signals;

	if( theApp->m_dcb.m_oSignalMap.count() > 0 )
	{
		Signals = theApp->m_dcb.m_oSignalMap.values( strId ) ;

To copy to clipboard, switch view to plain text mode 

What is the value of strId? It's smaller then m_oSignalMap.count()?

// we need to do the conversion here - TODO
				pSignal->m_strRawData = strData;  
				pSignal->m_nCount++;

// we need to do the conversion here - TODO
				pSignal->m_strRawData = strData;  
				pSignal->m_nCount++;

To copy to clipboard, switch view to plain text mode 

It's safe that pSignal isn't NULL?

[steg90]

Hi,

strId is set to "1911" for testing purposes, but sometimes it is a bad ptr and I don't know why?

Possible because this function gets called from another thread, well I say that, the other thread emits a signal which causes this code to be called, the other thread does pass across a pointer to the following data structure :

struct MESSAGEDATA
{
	QString strId;
	QString strData;
};

struct MESSAGEDATA
{
	QString strId;
	QString strData;
};

To copy to clipboard, switch view to plain text mode 

Thread just does :

MESSAGEDATA can[MAX];
 
		can[0].strId = "1911";
		can[0].strData = "0F0F0F0F0F0F";
		emit modifytable( &can[0] );

MESSAGEDATA can[MAX];

		can[0].strId = "1911";
		can[0].strData = "0F0F0F0F0F0F";
		emit modifytable( &can[0] );

To copy to clipboard, switch view to plain text mode 

Quite possible I've answered my own question, it could be the fact that passing a pointer to this structure is not safe?

And the modifytable slot function is :

void CanTree::updatetable( MESSAGEDATA* pData )
{
	m_pModel->updateTable( pData->strData, pData->strId );
}

void CanTree::updatetable( MESSAGEDATA* pData )
{
	m_pModel->updateTable( pData->strData, pData->strId );
}

To copy to clipboard, switch view to plain text mode 

Which updates the model with the relevant data.

Regards,
Steve

[freeskydiver]

void CanTree::updatetable( MESSAGEDATA* pData )
{
  try{
    m_pModel->updateTable( pData->strData, pData->strId );
  }catch(..){
  // check the pointer ...or you check him before ;)
}
}

void CanTree::updatetable( MESSAGEDATA* pData )
{
  try{
    m_pModel->updateTable( pData->strData, pData->strId );
  }catch(..){
  // check the pointer ...or you check him before ;)
}
}

To copy to clipboard, switch view to plain text mode 

You can try a TRY CATCH container to better see when the pointer is wrong.

[wysota]

If multiple threads are involved, they are likely to crash your application in a way you describe. Or you keep pointers to unexisting objects in the hash. You might want to use QPointer instead of simple pointers. At least you'll know if objects are still valid.

[steg90]

Hi,

Got this working now, no crashing, it was because I was passing a pointer to a structure which was created in my thread to another class outside of this thread.

I'm taking it that it is unwise to pass pointers to objects from one thread to another?

Regards,
Steve

[freeskydiver]

Hi,


I'm taking it that it is unwise to pass pointers to objects from one thread to another?

You must know what you to doing. It's sometimes no other way for exchange of data.

[wysota]

If those objects are QObjects then it's never safe unless you change the thread affinity at the same time, so that the object is moved to the other thread. But you can't access a QObject from two threads simoultaneously - you have to decide which is going to own the object. And then you can communicate with the object only with events (and signals/slots which use events).

[steg90]

Thanks for that, something to remember in the future.

[freeskydiver]

I see a small difference. You can use the same object from different threads. You must have a threadsafe call of the functions. This can you do with QMutex for example. Signals/Slots are very nice, but this is a Qt special. In C++ without Qt, you must communicate over threads, too.

[steg90]

Hi,

I did try using QMutex around the object in question, but to no avail, it still crashed...

The mutex will just stop the object from being modified by another thread while one thread is updating it. ( AKA Consumer / Producer problem ).

Regards,
Steve

[wysota]

I see a small difference. You can use the same object from different threads. You must have a threadsafe call of the functions. This can you do with QMutex for example. Signals/Slots are very nice, but this is a Qt special. In C++ without Qt, you must communicate over threads, too.

You can't guard an already existing class with mutexes. It's just not possible. You'd have to wrap all calls in all existing classes that call the object in critical sections. The problem is not that your function is not reentrant (because you are working around this particular problem when putting mutexes into your class) but that the object you call is not thread-safe and using mutexes in your code won't help unless you protect every other code that calls the same object. If you miss at least one, you're just calling for trouble (that's one of the problems with protecting code instead of data, by the way).

So it doesn't matter whether we're talking about Qt or non-Qt applications. If you don't protect everything, you're not protected at all.

[freeskydiver]

You can't guard an already existing class with mutexes. It's just not possible. You'd have to wrap all calls in all existing classes that call the object in critical sections.

Correct thats the only way, but what should I do? When I must use classes who are not thread safe. Ok it's a dangerous way, but possible.

[wysota]

It's not possible, if you can't overload every method that calls the object! And this is exactly the case.

[freeskydiver]

It's not possible, if you can't overload every method that calls the object! And this is exactly the case.

That's not essential. When I use a class, then I must encapsulate the calls of the not threadsafe class.

[wysota]

Yes, but all calls, not only the ones you make. If you use some kind of architecture (like Qt) it is possible that the architecture calls the objects (like QObjects), which is the exact case here. Thus you can't make QObjects thread-safe just by protecting your code.

[freeskydiver]

Upps.... you are right.
When the system call my object then can it crash.