QList::erase method does not work if I include an "emit signal" instruction

[d_stranz]

When you emit a signal, you return control to the Qt event loop. If you do not emit any signal from within checkTimeouts(), then your loops run completely to the end and checkTimeouts() exits before control returns to the event loop. When you emit the signal, then all pending events will be processed, including any new timeouts, which means you could be entering checkTimeouts() recursively. So basically, your list gets completely screwed up.

You can probably verify this by putting some qDebug() statements at the beginning and end of checkTimeouts() and see if you get an "entering checkTimeouts()" and "exiting checkTimeouts()" occurring together in pairs of enter / exit. If you get two enters in a row, you have a second timeout being processed before the first one has finished.

Time to re-think your design. Your basic problem is that it is taking longer to process all of the pending messages than the timeout period, so your timeouts are stacking up. So if your application allows you to ignore extra timeouts, then one easy way to avoid the problem is to keep a bool member variable in your PendingList class:

void PendingList::checkTimeouts()
{
 
    qDebug() << "Entering checkTimeouts()";
 
    if ( !inTimeout )
    {
        inTimeout = true;      // this is a member variable of PendingList; initialize it to false in the constructor
 
	qint64 t1 = QDateTime::currentMSecsSinceEpoch();
	for (int i = 0; i < MAX_CLIENTS; i++)
	{
		 QList<PendingMsg>::iterator it;
		 for (it = list[i].begin(); it != list[i].end(); )
		 {
			 quint64 t0 = it->request.getTimeStamp();
			 quint64 dift = t1 - t0;
			 if (dift >= it->timeout)
			 {		
				CMD cmd1 = it->cmd;
				CMD cmd2 = it->request.getCmd();
				it = list[i].erase(it);
				emitTimeout((System)i, cmd1, cmd2);
			 }
			 else
				 it++;
		 }
	}
	stopIfEmpty();
 
        inTimeout = false;
    }
 
    qDebug() << "Exiting checkTimeouts()";
}

void PendingList::checkTimeouts()
{

    qDebug() << "Entering checkTimeouts()";

    if ( !inTimeout )
    {
        inTimeout = true;      // this is a member variable of PendingList; initialize it to false in the constructor

	qint64 t1 = QDateTime::currentMSecsSinceEpoch();
	for (int i = 0; i < MAX_CLIENTS; i++)
	{
		 QList<PendingMsg>::iterator it;
		 for (it = list[i].begin(); it != list[i].end(); )
		 {
			 quint64 t0 = it->request.getTimeStamp();
			 quint64 dift = t1 - t0;
			 if (dift >= it->timeout)
			 {		
				CMD cmd1 = it->cmd;
				CMD cmd2 = it->request.getCmd();
				it = list[i].erase(it);
				emitTimeout((System)i, cmd1, cmd2);
			 }
			 else
				 it++;
		 }
	}
	stopIfEmpty();

        inTimeout = false;
    }

    qDebug() << "Exiting checkTimeouts()";
}

To copy to clipboard, switch view to plain text mode 

Another way to do it would be to stop the timer when you enter checkTimeouts() and start it when you exit. Or make it a single-shot, and simply restart it when you exit. That way you can be guaranteed that no new timeouts can be generated while you are processing the first one.

[amleto]

no one talking about the elephant? Why are you instantiating a raw array of QList? Which voice in your head thought that was a good idea?

[d_stranz]

no one talking about the elephant?

I thought it better to tackle things one step at a time. On the other hand, there's nothing basically wrong with declaring an array of QList<>. It will simply consist of an array of size MAX_CLIENTS of empty QList<> allocated on the stack. Nothing magic about that, and nothing that requires use of QVector< QList< PendingMsg > > or some other fancier data structure. Each QList<> in the array can be manipulated as if it were a standalone QList<>.

I am not sure about the copy semantics, though. I am not sure if accessing "list[i]" results in the return of a copy or of a reference. If it is a copy, then that would be pretty inefficient.

[wysota]

When you emit a signal, you return control to the Qt event loop. If you do not emit any signal from within checkTimeouts(), then your loops run completely to the end and checkTimeouts() exits before control returns to the event loop. When you emit the signal, then all pending events will be processed, including any new timeouts

No, that's not true, at least when a single execution thread is involved. The only way that the method is re-entered is that the signal is connected to a slot which calls this method again (e.g. emitTimeout() is connected to checkTimeouts()).

[d_stranz]

No, that's not true, at least when a single execution thread is involved. The only way that the method is re-entered is that the signal is connected to a slot which calls this method again (e.g. emitTimeout() is connected to checkTimeouts()).

Yes, of course you're right, and my little qDebug() test would have proven me wrong. Blame it on coffee - either not enough or too much.

So that seems to render my whole premise false; presuming that the OP isn't calling checkTimeouts() recursively, then why *does* emitting the signal cause the list to be improperly processed? Maybe one of the slots is also manipulating the list?

[wysota]

then why *does* emitting the signal cause the list to be improperly processed?

We don't know if the list is not processed correctly. We'd need to reproduce this errorneous behaviour which is not possible without a small compilable example. I'd start by disconnecting all the signals emitted from within emitTimeout() to see if the problem persists.