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