Non-Gui thread blocking gui

[Valheru]

The following code is a slot. When calling it using a signal from an object running in the GUI thread, the GUI blocks until the slot is finished executing. Which is a bit strange, because this code is running in it's own thread, so it shouldn't do that, right?
Also, it blocks until timeWaited == timeout*1000 (10 seconds in this case ) which it shouldn't, because the moment the thread connects to the server it changes the state of st into CONNECTED or LOGGED_IN. So obviously something is going wrong, but since this is my first time ever using threads I'm a bit lost.

void Connection::deferredWriteCommand( const QString& command )
{
    if( !isRunning() ){
        start();
    }
    bool done = false;
    int timeWaited = 0;
    while( !done && timeWaited < timeout*1000 ){
        switch( st ){
            case DISCONNECTED:
                wait( 1000 );
                timeWaited += 1000;
                break;
            case CONNECTED:
                if( parent->getAuthorization() ){
                    wait( 1000 );
                    timeWaited += 1000;
                }else{
                    socket->write( command.toLatin1(), (quint64) command.length() );
                    socket->flush();
                    done = true;
                }
                break;
            case LOGGED_IN:
                socket->write( command.toLatin1(), (quint64) command.length() );
                socket->flush();
                done = true;
                break;
            default:
                break;
        };
        if( done ){
            break;
        }
    }
}

void Connection::deferredWriteCommand( const QString& command )
{
    if( !isRunning() ){
        start();
    }
    bool done = false;
    int timeWaited = 0;
    while( !done && timeWaited < timeout*1000 ){
        switch( st ){
            case DISCONNECTED:
                wait( 1000 );
                timeWaited += 1000;
                break;
            case CONNECTED:
                if( parent->getAuthorization() ){
                    wait( 1000 );
                    timeWaited += 1000;
                }else{
                    socket->write( command.toLatin1(), (quint64) command.length() );
                    socket->flush();
                    done = true;
                }
                break;
            case LOGGED_IN:
                socket->write( command.toLatin1(), (quint64) command.length() );
                socket->flush();
                done = true;
                break;
            default:
                break;
        };
        if( done ){
            break;
        }
    }
}

To copy to clipboard, switch view to plain text mode 

[wysota]

What is this Connection class? Subclass of QThread? If so, then you're doing a strange thing here:

if( !isRunning() ){
    start();
}

if( !isRunning() ){
    start();
}

To copy to clipboard, switch view to plain text mode 

[Valheru]

Yes, it's a sublass of QThread. You're not allowed to run it from itself? Well, that never gets called anyway, since the code that starts the thread is called from another class, and looks like this :

if( !server.at( index )->connection( i )->isRunning() ){
            server.at( index )->connection( i )->start( QThread::NormalPriority );
        }

if( !server.at( index )->connection( i )->isRunning() ){
            server.at( index )->connection( i )->start( QThread::NormalPriority );
        }

To copy to clipboard, switch view to plain text mode 

[wysota]

You're not allowed to run it from itself?

It's not that you're not allowed. It just doesn't make sense to do so, because to trigger a slot, you need an event loop running, so if you run the loop from within a slot, the loop must be already running, because the slot was triggered.

In your case there are two possibilities - either the block occurs because you did something in the main thread or the slot is called in the context of the main thread (instead of the worker thread) which obviously blocks its event loop. Are you sure the slot is run from the worker thread? Could you show us the connect statement for that slot?

[Valheru]

void Server::fetchGroups()
{
    //emit emitLogData( "fetchGroups() called" );
    bool done = false;
    for( int i = 0; i < getNumberConnections(); i++ ){
      switch( connections.at( i )->state() ){
        case Connection::CONNECTED:
          //emit emitLogData( QString( "Connection %1 state = CONNECTED" ).arg( i ) );
          disconnect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ) );
          connect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ),
                    connections.at( i ), SLOT( deferredWriteCommand( const QString& ) ) );
          emit emitDeferredWriteCommand( "LIST\r\n" );
          done = true;
          break;
        case Connection::DISCONNECTED:
          //emit emitLogData( QString( "Connection %1 state = DISCONNECTED" ).arg( i ) );
          addServerTab();
          connections.at( i )->start( QThread::NormalPriority );
          disconnect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ) );
          connect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ),
                    connections.at( i ), SLOT( deferredWriteCommand( const QString& ) ) );
          emit emitDeferredWriteCommand( "LIST\r\n" );
          done = true;
          break;
        case Connection::LOGGED_IN:
          //emit emitLogData( QString( "Connection %1 state = LOGGED_IN" ).arg( i ) );
          connections.at( i )->writeCommand("LIST\r\n");
          done = true;
          break;
        case Connection::LISTING_GROUPS:
            break;
        default:
          emit emitLogData("No available connections" );
          break;
      };
      if( done ){
        //emit emitLogData( QString( "Connection %1 found" ).arg( i ) );
        break;
      }
    }
}

void Server::fetchGroups()
{
    //emit emitLogData( "fetchGroups() called" );
    bool done = false;
    for( int i = 0; i < getNumberConnections(); i++ ){
      switch( connections.at( i )->state() ){
        case Connection::CONNECTED:
          //emit emitLogData( QString( "Connection %1 state = CONNECTED" ).arg( i ) );
          disconnect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ) );
          connect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ),
                    connections.at( i ), SLOT( deferredWriteCommand( const QString& ) ) );
          emit emitDeferredWriteCommand( "LIST\r\n" );
          done = true;
          break;
        case Connection::DISCONNECTED:
          //emit emitLogData( QString( "Connection %1 state = DISCONNECTED" ).arg( i ) );
          addServerTab();
          connections.at( i )->start( QThread::NormalPriority );
          disconnect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ) );
          connect( this, SIGNAL( emitDeferredWriteCommand( const QString& ) ),
                    connections.at( i ), SLOT( deferredWriteCommand( const QString& ) ) );
          emit emitDeferredWriteCommand( "LIST\r\n" );
          done = true;
          break;
        case Connection::LOGGED_IN:
          //emit emitLogData( QString( "Connection %1 state = LOGGED_IN" ).arg( i ) );
          connections.at( i )->writeCommand("LIST\r\n");
          done = true;
          break;
        case Connection::LISTING_GROUPS:
            break;
        default:
          emit emitLogData("No available connections" );
          break;
      };
      if( done ){
        //emit emitLogData( QString( "Connection %1 found" ).arg( i ) );
        break;
      }
    }
}

To copy to clipboard, switch view to plain text mode 

[wysota]

Hmm... very strange... Why don't you use custom events instead of such a complicated and strange design?

const int MyCustomEventNum = QEvent::User+1;
class MyCustomEvent : public QEvent {
  MyCustomEvent(const QString &comm) : QEvent(MyCustomEventNum){
    _comm = comm;
  }
  const QString &command() const { return _comm; }
private:
  QString _comm;
};
void Connection::customEvent(QEvent *e){
  if(e->type()==MyCustomEventNum){
    doSomething(((MyCustomEvent*)e)->command());
  }
}

const int MyCustomEventNum = QEvent::User+1;
class MyCustomEvent : public QEvent {
  MyCustomEvent(const QString &comm) : QEvent(MyCustomEventNum){
    _comm = comm;
  }
  const QString &command() const { return _comm; }
private:
  QString _comm;
};
void Connection::customEvent(QEvent *e){
  if(e->type()==MyCustomEventNum){
    doSomething(((MyCustomEvent*)e)->command());
  }
}

To copy to clipboard, switch view to plain text mode 

and in your code:

case Connection::CONNECTED:
  QCoreApplication::postEvent(connections.at( i ), new MyCustomEvent("LIST\r\n"));
break;

case Connection::CONNECTED:
  QCoreApplication::postEvent(connections.at( i ), new MyCustomEvent("LIST\r\n"));
break;

To copy to clipboard, switch view to plain text mode 

[Valheru]

Well, the reason for this is because the user could concievably call fetchGroups() before connecting to the server. In this case, the first available Connection should connect to the server before issuing the command to the NNTP server. This is why I created a slot in the Connection class that does just that. What is confusing me is that although every Connection runs in it's own thread, and doesn't block the GUI while it's doing other things, it does block the GUI when doing this one thing.

[wysota]

You could try explicitely calling connect with the additional parameter Qt::QueuedConnection, but I still think using custom events would be simpler, quicker and nicer.

[Valheru]

Heh, I just tried it as a queuedconnection and you're right - it didn't help I've just been looking at the docs for the custom event, I didn't even know that they existed - thanks for the tip. I'll try that now.

[wysota]

It may behave incorrectly because you are emitting the signal to the thread object. And the thread object lives in the same thread as the calling object (only objects created within run() or from within methods called from the thread's event loop live in the worker thread), so the slot might be called by the wrong (main) thread. It could be possible that all you need to do is to wait until the thread is started before emitting the signal (but you have to explicitely use queued connections for that to work!) - that would explain why subsequent signals work correctly.

connections.at( i )->start( QThread::NormalPriority );
while(!connections.at(i)->isRunning()) QThread::sleep(100); //crude, but should work

connections.at( i )->start( QThread::NormalPriority );
while(!connections.at(i)->isRunning()) QThread::sleep(100); //crude, but should work

To copy to clipboard, switch view to plain text mode 

[Valheru]

Would the custom event run in the worker thread? I'd prefer to do it that way if it is so, I'm busy restructuring the program now to make it cleaner and fix bugs so I don't want to hack some half-assed fix in if at all possible

[wysota]

Would the custom event run in the worker thread?

Yes, but with the same condition - make sure the thread is already running its event queue (as this probably moves the thread object to the thread itself) before posting the first event.

[Valheru]

Hmm, it doesn't seem to like being passed an int as argument for the creator of QEvent. Would it be safe to statically cast it to QEvent::Type?

connection.h:35: error: invalid conversion from ‘int’ to ‘QEvent::Type’

connection.h:35: error: invalid conversion from ‘int’ to ‘QEvent::Type’

To copy to clipboard, switch view to plain text mode 

[jpn]

In Qt 4, a slot in a QThread subclass ends up being executed in the thread where the QThread object itself is living in (QObject lives in the thread where it was created).

A signal and slot connection is direct between objects living in the same thread. So if a QThread object is instantiated in the main GUI thread, the QThread object itself also lives in the main GUI thread. And so the connection is direct and the slot in a QThread subclass ends up being executed in the main GUI thread aswell.

You can avoid all this by simply creating a QObject in QThread::run() and connecting signals to that object instead of the QThread object. This ensures that the signal slot connection is across thread.

[jpn]

The behaviour is exactly the same with posting events. The event gets processed in the event loop running in the thread where the receiver object lives in. Since the QThread object lives in the main thread, it's customEvent() gets executed in the main thread as well.

[wysota]

Hmm, it doesn't seem to like being passed an int as argument for the creator of QEvent. Would it be safe to statically cast it to QEvent::Type?

Yes.

The behaviour is exactly the same with posting events. The event gets processed in the event loop running in the thread where the receiver object lives in. Since the QThread object lives in the main thread, it's customEvent() gets executed in the main thread as well.

Hmm... I thought exec() moves the object into the thread. If it doesn't, it should be enough to move it inside run():

void MyThread::run(){
  moveToThread(this);
  exec();
}

void MyThread::run(){
  moveToThread(this);
  exec();
}

To copy to clipboard, switch view to plain text mode 

[jpn]

Hmm... I thought exec() moves the object into the thread. If it doesn't, it should be enough to move it inside run():

void MyThread::run(){
  moveToThread(this);
  exec();
}

void MyThread::run(){
  moveToThread(this);
  exec();
}

To copy to clipboard, switch view to plain text mode 

Good idea, but seems to be not possible:

QObject::moveToThread: Current thread (003E7CA0) is not the object's thread (0012FEC4).
Cannot move to target thread (0012FEC4)

I tested with both

moveToThread(this);
// and
moveToThread(QThread::currentThread());

moveToThread(this);
// and
moveToThread(QThread::currentThread());

To copy to clipboard, switch view to plain text mode 

[chca]

Good idea, but seems to be not possible:


I tested with both

moveToThread(this);
// and
moveToThread(QThread::currentThread());

moveToThread(this);
// and
moveToThread(QThread::currentThread());

To copy to clipboard, switch view to plain text mode 

I had the same problem, but when you call "moveToThread" in the class' constructor (which is executed in the thread the object is created in), it seems to work.