QUrlOperator::copy sends two finished signals

[hvengel]

I am using QUrlOperator::copy to copy files as part of setting up user configuration information the first time the app is run. I had this setup as part of the main() function and was running it before I started any dialogs. Since QUrlOperator::copy runs asynchronously I have been concerned that it might be possible that the files might not have been copied by the time the first dialog opens. In the past there were 4 fairly small files being copied and it has not been an issue. But current development code now has 6 files and soon another 2 will be added. Some users testing with current development code are now starting to experience problems with this and I am in the process of setting this up so that the first dialog blocks before it needs the files until the file copy operation is complete.

I have this working but I am seeing some strange problems that I don't understand. The basic issue is that I am seeing 2 finished signals for each copy operation and not 1 like I had expected.

My code looks like this:

Globals

static QUrlOperator urlOpTempFile;
static QMutex mutexTempFile;
static int copyNumTempFile = 0;

static QUrlOperator urlOpTempFile;
static QMutex mutexTempFile;
static int copyNumTempFile = 0;

To copy to clipboard, switch view to plain text mode 

Slot to handle finished signal

void lprofMain::templateFileCopied( QNetworkOperation *op )
{
    mutexTempFile.lock();
    if ( !op )
        return;
    if ( op->state() == QNetworkProtocol::StFailed )
    {
        QMessageBox::critical( this, tr( "ERROR" ), op->protocolDetail() );
    }
    else if(op->state() == QNetworkProtocol::StDone)
    {
        copyNumTempFile = copyNumTempFile - 1;
    }
    mutexTempFile.unlock();
}

void lprofMain::templateFileCopied( QNetworkOperation *op )
{
    mutexTempFile.lock();
    if ( !op )
        return;
    if ( op->state() == QNetworkProtocol::StFailed )
    {
        QMessageBox::critical( this, tr( "ERROR" ), op->protocolDetail() );
    }
    else if(op->state() == QNetworkProtocol::StDone)
    {
        copyNumTempFile = copyNumTempFile - 1;
    }
    mutexTempFile.unlock();
}

To copy to clipboard, switch view to plain text mode 

Routine that starts the copy operations with a "fix"

void lprofMain::Create_Config_Dir(const QString data_path)
{
 
        // Copy the template files into the users config directory
        QDir d (data_path + QString("/template/"));
        d.setFilter(QDir::Files | QDir::Hidden);
        const QFileInfoList *list = d.entryInfoList();
        QFileInfoListIterator it( *list );
        QFileInfo *fi;
        while ( (fi = it.current()) != 0 )
        {
            ++it;
            if (fi->fileName().contains("ITX", FALSE))
            {
                mutexTempFile.lock();
                copyNumTempFile = copyNumTempFile + 1;
                urlOpTempFile.copy( data_path + QString("/template/") +
                        fi->fileName(), dir->homeDirPath() +
                        QString("/.lprof/templates/") );
                // next line is the "fix"
                copyNumTempFile = copyNumTempFile + 1;
                mutexTempFile.unlock();
            }
        }
    }
}

void lprofMain::Create_Config_Dir(const QString data_path)
{
    
        // Copy the template files into the users config directory
        QDir d (data_path + QString("/template/"));
        d.setFilter(QDir::Files | QDir::Hidden);
        const QFileInfoList *list = d.entryInfoList();
        QFileInfoListIterator it( *list );
        QFileInfo *fi;
        while ( (fi = it.current()) != 0 )
        {
            ++it;
            if (fi->fileName().contains("ITX", FALSE))
            {
                mutexTempFile.lock();
                copyNumTempFile = copyNumTempFile + 1;
                urlOpTempFile.copy( data_path + QString("/template/") +
                        fi->fileName(), dir->homeDirPath() +
                        QString("/.lprof/templates/") );
                // next line is the "fix"
                copyNumTempFile = copyNumTempFile + 1;
                mutexTempFile.unlock();
            }
        }
    }
}

To copy to clipboard, switch view to plain text mode 

main dialog creation code

lprofMain::lprofMain( QWidget* parent)
    : lprofMainBase( parent, "", 0,)
{
    QObject::connect( &urlOpTempFile, SIGNAL( finished( QNetworkOperation *) ), this, SLOT( templateFileCopied( QNetworkOperation * ) ) );
 
    Create_Config_Dir(QDir::currentDirPath());
 
    while (copyNumTempFile>0)
    {
        lprofApp -> processEvents();
    }
}

lprofMain::lprofMain( QWidget* parent)
    : lprofMainBase( parent, "", 0,)
{
    QObject::connect( &urlOpTempFile, SIGNAL( finished( QNetworkOperation *) ), this, SLOT( templateFileCopied( QNetworkOperation * ) ) );

    Create_Config_Dir(QDir::currentDirPath());

    while (copyNumTempFile>0)
    {
        lprofApp -> processEvents();
    }
}

To copy to clipboard, switch view to plain text mode 

I am running Qt 3.3.8 on a Linux box. Anyone seen this type of thing before or have any ideas about what could be causing this? The code above works but my main concern is that it does not look right to me and I am concerned that I may be "fixing" something that is really a problem with the particular version of Qt I am running.

[hvengel]

Bump. Sorry I didn't intend this to stump the band.

[jacek]

Anyone seen this type of thing before or have any ideas about what could be causing this?

As the docs say:

... a move or copy operation consists of multiple operations (get(), put() and maybe remove()), this function doesn't return a single QNetworkOperation, but rather a list of them. They are in the order: get(), put() and (if applicable) remove().

So it looks like finished() is emitted for each of those operations.

You could try something like this:

...
QPtrList<QNetworkOperation> list = urlOpTempFile.copy( ... );
lastOp = list.last();
...
 
void lprofMain::templateFileCopied( QNetworkOperation *op )
{
    if ( !op )
        return;
    if ( op->state() == QNetworkProtocol::StFailed )
    {
        QMessageBox::critical( this, tr( "ERROR" ), op->protocolDetail() );
    }
    else if( op == lastOp && op->state() == QNetworkProtocol::StDone )
    {
        QMutexLocker locker( & mutexTempFile );
        copyNumTempFile = copyNumTempFile - 1;
    }
}

...
QPtrList<QNetworkOperation> list = urlOpTempFile.copy( ... );
lastOp = list.last();
...

void lprofMain::templateFileCopied( QNetworkOperation *op )
{
    if ( !op )
        return;
    if ( op->state() == QNetworkProtocol::StFailed )
    {
        QMessageBox::critical( this, tr( "ERROR" ), op->protocolDetail() );
    }
    else if( op == lastOp && op->state() == QNetworkProtocol::StDone )
    {
        QMutexLocker locker( & mutexTempFile );
        copyNumTempFile = copyNumTempFile - 1;
    }
}

To copy to clipboard, switch view to plain text mode 

[hvengel]

I think the important things is that that there are more than on finished signal for each QUrlOperator::copy. Likely one for get() and put() so having two signals looks like it is correct.

I don't think your code will work however since there a likely more than one of these QUrlOperator::copy threads running at a given time and the finished events will not happen in a known order. Your proposed fix assumes that these are ordered and as a result could fail.

Since it appears that two finished signals is normal for a QUrlOperator::copy then it looks to me like my algorithms are basically correct.

[jacek]

Since it appears that two finished signals is normal for a QUrlOperator::copy then it looks to me like my algorithms are basically correct.

Yes, they are and I never wrote that they're wrong (actually I simply overlooked that "fix" line in your code). The only thing is that the meaning of copyNumTempFile changes a bit, but it doesn't seem to be a problem.

Although there is an error in lprofMain::templateFileCopied() --- check what happens with the mutex if that slot was for some reason invoked with a null parameter.

I don't think your code will work however since there a likely more than one of these QUrlOperator::copy threads running at a given time and the finished events will not happen in a known order. Your proposed fix assumes that these are ordered and as a result could fail.

If Qt 3 could use more than one thread to handle those operations, then of course you would be right. Unfortunately all network related classes in Qt 3 are not thread safe, so one can assume that QNetworkProtocol doesn't change the order of network operations.

[hvengel]

You are right it would not release the lock and this would cause a deadlock condition. Thanks for pointing that out.

After doing more research I now have this working the way it should. The signal handler has been changed to look like this:

void lprofMain::templateFileCopied( QNetworkOperation *op )
{    
    if ( !op )
        return;
 
    if ( op->state() == QNetworkProtocol::StFailed )
    {
        QMessageBox::critical( this, tr( "ERROR" ), op->protocolDetail() );
    }
    else if(op->operation () == QNetworkProtocol::OpPut )
    {
        mutexTempFile.lock();
        copyNumTempFile = copyNumTempFile - 1;
        mutexTempFile.unlock();
    }
 
}

void lprofMain::templateFileCopied( QNetworkOperation *op )
{    
    if ( !op )
        return;
    
    if ( op->state() == QNetworkProtocol::StFailed )
    {
        QMessageBox::critical( this, tr( "ERROR" ), op->protocolDetail() );
    }
    else if(op->operation () == QNetworkProtocol::OpPut )
    {
        mutexTempFile.lock();
        copyNumTempFile = copyNumTempFile - 1;
        mutexTempFile.unlock();
    }
    
}

To copy to clipboard, switch view to plain text mode 

So it now only decrements the counter if it is a put operation that has finished. And in the code where I start the QUrlOperator::copy running I only increment the counter one time for each QUrlOperator::copy thread. I have also reduced the scope of the QMutex lock so that it only protects the counter operations.