[kdevplatform] project: Don't let the ProjectModel to be accessed from different threads

Aleix Pol aleixpol at kde.org
Tue Sep 10 23:09:14 UTC 2013


Git commit 010d7bbb3f157faacdc65bab114d6e74c99afd08 by Aleix Pol.
Committed on 10/09/2013 at 22:45.
Pushed by apol into branch 'master'.

Don't let the ProjectModel to be accessed from different threads

Experience showed that it didn't work that well, especially since
there's no aboutToChangeData().

CMake was the only user of the feature AFAIK, so this is the best
behavior I'd say.

CCMAIL: kdevelop-devel at kde.org

M  +7    -21   project/projectmodel.cpp
M  +0    -61   project/tests/projectmodeltest.cpp
M  +0    -1    project/tests/projectmodeltest.h

http://commits.kde.org/kdevplatform/010d7bbb3f157faacdc65bab114d6e74c99afd08

diff --git a/project/projectmodel.cpp b/project/projectmodel.cpp
index d753e75..0a05eaa 100644
--- a/project/projectmodel.cpp
+++ b/project/projectmodel.cpp
@@ -42,20 +42,6 @@
 #include <QMetaClassInfo>
 #include <QThread>
 
-// Utility function to determine between direct connection and blocking-queued-connection
-// for emitting of signals for data changes/row addition/removal
-// BlockingQueuedConnection is necessary here as slots connected to the "aboutToBe" signals
-// expect the actual model content to not have changed yet. So we need to make sure the
-// signal is delivered before we really do something.
-static Qt::ConnectionType getConnectionTypeForSignalDelivery( KDevelop::ProjectModel* model )
-{
-    if( QThread::currentThread() == model->thread() ) {
-        return Qt::DirectConnection;
-    } else {
-        return Qt::BlockingQueuedConnection;
-    }
-}
-
 namespace KDevelop
 {
 
@@ -199,7 +185,7 @@ ProjectBaseItem* ProjectBaseItem::takeRow(int row)
     Q_ASSERT(row >= 0 && row < d->children.size());
     
     if( model() ) {
-        QMetaObject::invokeMethod( model(), "rowsAboutToBeRemoved", getConnectionTypeForSignalDelivery( model() ), Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row) );
+        QMetaObject::invokeMethod( model(), "rowsAboutToBeRemoved", Qt::DirectConnection, Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row) );
     }
     ProjectBaseItem* olditem = d->children.takeAt( row );
     olditem->d_func()->parent = 0;
@@ -212,7 +198,7 @@ ProjectBaseItem* ProjectBaseItem::takeRow(int row)
     }
     
     if( model() ) {
-        QMetaObject::invokeMethod( model(), "rowsRemoved", getConnectionTypeForSignalDelivery( model() ), Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row) );
+        QMetaObject::invokeMethod( model(), "rowsRemoved", Qt::DirectConnection, Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row) );
     }
     return olditem;
 }
@@ -232,7 +218,7 @@ void ProjectBaseItem::removeRows(int row, int count)
     Q_ASSERT(row >= 0 && row + count <= d->children.size());
 
     if( model() ) {
-        QMetaObject::invokeMethod( model(), "rowsAboutToBeRemoved", getConnectionTypeForSignalDelivery( model() ), Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row + count - 1) );
+        QMetaObject::invokeMethod( model(), "rowsAboutToBeRemoved", Qt::DirectConnection, Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row + count - 1) );
     }
 
     //NOTE: we unset parent, row and model manually to speed up the deletion
@@ -260,7 +246,7 @@ void ProjectBaseItem::removeRows(int row, int count)
     }
 
     if( model() ) {
-        QMetaObject::invokeMethod( model(), "rowsRemoved", getConnectionTypeForSignalDelivery( model() ), Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row + count - 1) );
+        QMetaObject::invokeMethod( model(), "rowsRemoved", Qt::DirectConnection, Q_ARG(QModelIndex, index()), Q_ARG(int, row), Q_ARG(int, row + count - 1) );
     }
 }
 
@@ -350,7 +336,7 @@ void ProjectBaseItem::setText( const QString& text )
     d->text = text;
     if( d->model ) {
         QModelIndex idx = index();
-        QMetaObject::invokeMethod( d->model, "dataChanged", getConnectionTypeForSignalDelivery( d->model ), Q_ARG(QModelIndex, idx), Q_ARG(QModelIndex, idx) );
+        QMetaObject::invokeMethod( d->model, "dataChanged", Qt::DirectConnection, Q_ARG(QModelIndex, idx), Q_ARG(QModelIndex, idx) );
     }
 }
 
@@ -429,14 +415,14 @@ void ProjectBaseItem::appendRow( ProjectBaseItem* item )
     int startrow,endrow;
     if( model() ) {
         startrow = endrow = d->children.count();
-        QMetaObject::invokeMethod( model(), "rowsAboutToBeInserted", getConnectionTypeForSignalDelivery( model() ), Q_ARG(QModelIndex, index()), Q_ARG(int, startrow), Q_ARG(int, endrow) );
+        QMetaObject::invokeMethod( model(), "rowsAboutToBeInserted", Qt::DirectConnection, Q_ARG(QModelIndex, index()), Q_ARG(int, startrow), Q_ARG(int, endrow) );
     }
     d->children.append( item );
     item->setRow( d->children.count() - 1 );
     item->d_func()->parent = this;
     item->setModel( model() );
     if( model() ) {
-        QMetaObject::invokeMethod( model(), "rowsInserted", getConnectionTypeForSignalDelivery( model() ), Q_ARG( QModelIndex, index() ), Q_ARG( int, startrow ), Q_ARG( int, endrow ) );
+        QMetaObject::invokeMethod( model(), "rowsInserted", Qt::DirectConnection, Q_ARG( QModelIndex, index() ), Q_ARG( int, startrow ), Q_ARG( int, endrow ) );
     }
 }
 
diff --git a/project/tests/projectmodeltest.cpp b/project/tests/projectmodeltest.cpp
index c8cadb2..e173333 100644
--- a/project/tests/projectmodeltest.cpp
+++ b/project/tests/projectmodeltest.cpp
@@ -43,52 +43,6 @@ using KDevelop::ProjectBuildFolderItem;
 
 using KDevelop::TestProject;
 
-class AddItemThread : public QThread
-{
-Q_OBJECT
-public:
-    AddItemThread( ProjectBaseItem* _parentItem, QObject* parent = 0 )
-        : QThread( parent ), parentItem( _parentItem )
-    {
-    }
-    virtual void run()
-    {
-        this->sleep( 1 );
-        KUrl url = parentItem->url();
-        url.addPath("folder1");
-        ProjectFolderItem* folder = new ProjectFolderItem( 0, url, parentItem );
-        url.addPath( "file1" );
-        new ProjectFileItem( 0, url, folder );
-        emit addedItems();
-    }
-signals:
-    void addedItems();
-private:
-    ProjectBaseItem* parentItem;
-};
-
-class SignalReceiver : public QObject
-{
-Q_OBJECT
-public:
-    SignalReceiver(ProjectModel* _model, QObject* parent = 0)
-        : QObject(parent), model( _model )
-    {
-    }
-    QThread* threadOfSignalEmission() const
-    {
-        return threadOfReceivedSignal;
-    }
-private slots:
-    void rowsInserted( const QModelIndex&, int, int )
-    {
-        threadOfReceivedSignal = QThread::currentThread();
-    }
-private:
-    QThread* threadOfReceivedSignal;
-    ProjectModel* model;
-};
-
 void debugItemModel(QAbstractItemModel* m, const QModelIndex& parent=QModelIndex(), int depth=0)
 {
     Q_ASSERT(m);
@@ -509,19 +463,6 @@ void ProjectModelTest::testWithProject()
     QCOMPARE( item->url(), proj->folder() );
 }
 
-void ProjectModelTest::testAddItemInThread()
-{
-    ProjectFolderItem* root = new ProjectFolderItem( 0, KUrl("file:///f1"), 0 );
-    model->appendRow( root );
-    AddItemThread t( root );
-    SignalReceiver check( model );
-    connect( model, SIGNAL(rowsInserted(QModelIndex,int,int)), &check, SLOT(rowsInserted(QModelIndex,int,int)), Qt::DirectConnection );
-    KDevelop::KDevSignalSpy spy( &t, SIGNAL(addedItems()), Qt::QueuedConnection );
-    t.start();
-    QVERIFY(spy.wait( 10000 ));
-    QCOMPARE( qApp->thread(), check.threadOfSignalEmission() );
-}
-
 void ProjectModelTest::testItemsForUrl()
 {
     QFETCH(KUrl, url);
@@ -610,5 +551,3 @@ void ProjectModelTest::testProjectFileIcon()
 }
 
 QTEST_KDEMAIN( ProjectModelTest, GUI)
-#include "projectmodeltest.moc"
-#include "moc_projectmodeltest.cpp"
diff --git a/project/tests/projectmodeltest.h b/project/tests/projectmodeltest.h
index 5601f87..816bccb 100644
--- a/project/tests/projectmodeltest.h
+++ b/project/tests/projectmodeltest.h
@@ -49,7 +49,6 @@ private slots:
     void testChangeWithProxyModel();
     void testWithProject();
     void testTakeRow();
-    void testAddItemInThread();
     void testItemsForUrl();
     void testItemsForUrl_data();
     void testProjectProxyModel();


More information about the KDevelop-devel mailing list