Skip to content

Commit

Permalink
safely delete browser items if createChildren() is running in thread
Browse files Browse the repository at this point in the history
  • Loading branch information
blazek committed Dec 15, 2014
1 parent d28619d commit 138d836
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 31 deletions.
116 changes: 87 additions & 29 deletions src/core/qgsdataitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,18 +160,58 @@ QgsDataItem::QgsDataItem( QgsDataItem::Type type, QgsDataItem* parent, QString n
: QObject()
, mType( type )
, mCapabilities( NoCapabilities )
, mParent( parent )
, mParent( 0 ) // items are created without parent, parent is set later by setParent()
, mState( NotPopulated )
, mPopulated( false )
, mName( name )
, mPath( path )
, mDeferredDelete( false )
, mWatcher( 0 )
{
Q_UNUSED( parent );
}

QgsDataItem::~QgsDataItem()
{
QgsDebugMsgLevel( "mName = " + mName + " mPath = " + mPath, 2 );
QgsDebugMsgLevel( QString( "mName = %1 mPath = %2 mChildren.size() = %3" ).arg( mName ).arg( mPath ).arg( mChildren.size() ) , 2 );
foreach ( QgsDataItem *child, mChildren )
{
if ( !child ) // should not happen
continue;
child->deleteLater();
}
mChildren.clear();

if ( mWatcher && !mWatcher->isFinished() )
{
// this should not usually happen (until the item was deleted directly when createChildren was running)
QgsDebugMsg( "mWatcher not finished (should not happen) -> waitForFinished()" );
mDeferredDelete = true;
mWatcher->waitForFinished();
}
}

void QgsDataItem::deleteLater()
{
QgsDebugMsg( "path = " + path() );
setParent( 0 ); // also disconnects parent
foreach ( QgsDataItem *child, mChildren )
{
if ( !child ) // should not happen
continue;
child->deleteLater();
}
mChildren.clear();

if ( mWatcher && !mWatcher->isFinished() )
{
QgsDebugMsg( "mWatcher not finished -> schedule to delete later" );
mDeferredDelete = true;
}
else
{
QObject::deleteLater();
}
}

QIcon QgsDataItem::icon()
Expand Down Expand Up @@ -278,6 +318,14 @@ QVector<QgsDataItem*> QgsDataItem::runCreateChildren( QgsDataItem* item )
void QgsDataItem::childrenCreated()
{
QgsDebugMsg( QString( "path = %1 children.size() = %2" ).arg( path() ).arg( mWatcher->result().size() ) );

if ( deferredDelete() )
{
QgsDebugMsg( "Item was scheduled to be deleted later" );
QObject::deleteLater();
return;
}

if ( mChildren.size() == 0 ) // usually populating but may also be refresh if originaly there were no children
{
populate( mWatcher->result() );
Expand Down Expand Up @@ -376,7 +424,7 @@ void QgsDataItem::refresh( QVector<QgsDataItem*> children )
mChildren.value( index )->refresh( child->children() );
}

delete child;
child->deleteLater();
continue;
}
addChildItem( child, true );
Expand All @@ -393,6 +441,41 @@ bool QgsDataItem::hasChildren()
return ( state() == Populated ? mChildren.count() > 0 : true );
}

void QgsDataItem::setParent( QgsDataItem* parent )
{
if ( mParent )
{
disconnect( this, SIGNAL( beginInsertItems( QgsDataItem*, int, int ) ),
mParent, SLOT( emitBeginInsertItems( QgsDataItem*, int, int ) ) );
disconnect( this, SIGNAL( endInsertItems() ),
mParent, SLOT( emitEndInsertItems() ) );
disconnect( this, SIGNAL( beginRemoveItems( QgsDataItem*, int, int ) ),
mParent, SLOT( emitBeginRemoveItems( QgsDataItem*, int, int ) ) );
disconnect( this, SIGNAL( endRemoveItems() ),
mParent, SLOT( emitEndRemoveItems() ) );
disconnect( this, SIGNAL( dataChanged( QgsDataItem* ) ),
mParent, SLOT( emitDataChanged( QgsDataItem* ) ) );
disconnect( this, SIGNAL( stateChanged( QgsDataItem*, QgsDataItem::State ) ),
mParent, SLOT( emitStateChanged( QgsDataItem*, QgsDataItem::State ) ) );
}
if ( parent )
{
connect( this, SIGNAL( beginInsertItems( QgsDataItem*, int, int ) ),
parent, SLOT( emitBeginInsertItems( QgsDataItem*, int, int ) ) );
connect( this, SIGNAL( endInsertItems() ),
parent, SLOT( emitEndInsertItems() ) );
connect( this, SIGNAL( beginRemoveItems( QgsDataItem*, int, int ) ),
parent, SLOT( emitBeginRemoveItems( QgsDataItem*, int, int ) ) );
connect( this, SIGNAL( endRemoveItems() ),
parent, SLOT( emitEndRemoveItems() ) );
connect( this, SIGNAL( dataChanged( QgsDataItem* ) ),
parent, SLOT( emitDataChanged( QgsDataItem* ) ) );
connect( this, SIGNAL( stateChanged( QgsDataItem*, QgsDataItem::State ) ),
parent, SLOT( emitStateChanged( QgsDataItem*, QgsDataItem::State ) ) );
}
mParent = parent;
}

void QgsDataItem::addChildItem( QgsDataItem * child, bool refresh )
{
QgsDebugMsg( QString( "path = %1 add child #%2 - %3 - %4" ).arg( mPath ).arg( mChildren.size() ).arg( child->mName ).arg( child->mType ) );
Expand Down Expand Up @@ -423,19 +506,6 @@ void QgsDataItem::addChildItem( QgsDataItem * child, bool refresh )
mChildren.insert( i, child );
child->setParent( this );

connect( child, SIGNAL( beginInsertItems( QgsDataItem*, int, int ) ),
this, SLOT( emitBeginInsertItems( QgsDataItem*, int, int ) ) );
connect( child, SIGNAL( endInsertItems() ),
this, SLOT( emitEndInsertItems() ) );
connect( child, SIGNAL( beginRemoveItems( QgsDataItem*, int, int ) ),
this, SLOT( emitBeginRemoveItems( QgsDataItem*, int, int ) ) );
connect( child, SIGNAL( endRemoveItems() ),
this, SLOT( emitEndRemoveItems() ) );
connect( child, SIGNAL( dataChanged( QgsDataItem* ) ),
this, SLOT( emitDataChanged( QgsDataItem* ) ) );
connect( child, SIGNAL( stateChanged( QgsDataItem*, QgsDataItem::State ) ),
this, SLOT( emitStateChanged( QgsDataItem*, QgsDataItem::State ) ) );

if ( refresh )
emit endInsertItems();
}
Expand All @@ -446,7 +516,7 @@ void QgsDataItem::deleteChildItem( QgsDataItem * child )
Q_ASSERT( i >= 0 );
emit beginRemoveItems( this, i, i );
mChildren.remove( i );
delete child; // deleting QObject child removes it from QObject parent
child->deleteLater();
emit endRemoveItems();
}

Expand All @@ -458,18 +528,6 @@ QgsDataItem * QgsDataItem::removeChildItem( QgsDataItem * child )
emit beginRemoveItems( this, i, i );
mChildren.remove( i );
emit endRemoveItems();
disconnect( child, SIGNAL( beginInsertItems( QgsDataItem*, int, int ) ),
this, SLOT( emitBeginInsertItems( QgsDataItem*, int, int ) ) );
disconnect( child, SIGNAL( endInsertItems() ),
this, SLOT( emitEndInsertItems() ) );
disconnect( child, SIGNAL( beginRemoveItems( QgsDataItem*, int, int ) ),
this, SLOT( emitBeginRemoveItems( QgsDataItem*, int, int ) ) );
disconnect( child, SIGNAL( endRemoveItems() ),
this, SLOT( emitEndRemoveItems() ) );
disconnect( child, SIGNAL( dataChanged( QgsDataItem* ) ),
this, SLOT( emitDataChanged( QgsDataItem* ) ) );
disconnect( child, SIGNAL( stateChanged( QgsDataItem*, QgsDataItem::State ) ),
this, SLOT( emitStateChanged( QgsDataItem*, QgsDataItem::State ) ) );
child->setParent( 0 );
return child;
}
Expand Down
28 changes: 26 additions & 2 deletions src/core/qgsdataitem.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class CORE_EXPORT QgsDataItem : public QObject
Favourites
};

/** Create new data item.
* @param parent - the param is ignored, children created by createChildren() do not have parent, parent is set later by setParent if the item really becomes a parent child */
QgsDataItem( QgsDataItem::Type type, QgsDataItem* parent, QString name, QString path );
virtual ~QgsDataItem();

Expand All @@ -62,7 +64,8 @@ class CORE_EXPORT QgsDataItem : public QObject

virtual void refresh();

// Create vector of children
/** Create children. Children are not expected to have parent set.
* This method MUST BE THREAD SAFE. */
virtual QVector<QgsDataItem*> createChildren();

// Populate children using children vector created by createChildren()
Expand Down Expand Up @@ -139,8 +142,12 @@ class CORE_EXPORT QgsDataItem : public QObject
// members

Type type() const { return mType; }
/** Get item parent. QgsDataItem maintains its own items hierarchy, it does not use
* QObject hierarchy. */
QgsDataItem* parent() const { return mParent; }
void setParent( QgsDataItem* parent ) { mParent = parent; }
/** Set item parent and connect / disconnect parent to / from item signals.
* It does not add itself to parents children (mChildren) */
void setParent( QgsDataItem* parent );
QVector<QgsDataItem*> children() const { return mChildren; }
virtual QIcon icon();
QString name() const { return mName; }
Expand All @@ -160,6 +167,13 @@ class CORE_EXPORT QgsDataItem : public QObject
virtual void populate( QVector<QgsDataItem*> children );
virtual void refresh( QVector<QgsDataItem*> children );
QIcon populatingIcon() { return mPopulatingIcon; }
/** The item is scheduled to be deleted. E.g. if deleteLater() is called when
* item is in Populating state (createChildren() running in another thread),
* the deferredDelete() returns true and item will be deleted once Populating finished.
* Items with slow reateChildren() (for example network or database based) may
* check during createChildren() if deferredDelete() returns true and return from
* createChildren() immediately because result will be useless. */
bool deferredDelete() { return mDeferredDelete; }

Type mType;
Capabilities mCapabilities;
Expand All @@ -180,6 +194,14 @@ class CORE_EXPORT QgsDataItem : public QObject
static QMap<QString, QIcon> mIconMap;

public slots:
/** Safely delete the item:
* - disconnects parent
* - unsets parent (but does not remove itself)
* - deletes all its descendants recursively
* - waits until Populating state (createChildren() in thread) finished without blocking main thread
* - calls QObject::deleteLater()
*/
virtual void deleteLater();
void emitBeginInsertItems( QgsDataItem* parent, int first, int last );
void emitEndInsertItems();
void emitBeginRemoveItems( QgsDataItem* parent, int first, int last );
Expand All @@ -201,6 +223,8 @@ class CORE_EXPORT QgsDataItem : public QObject
private:
static QVector<QgsDataItem*> runCreateChildren( QgsDataItem* item );

// Set to true if object has to be deleted when possible (nothing running in threads)
bool mDeferredDelete;
QFutureWatcher< QVector <QgsDataItem*> > *mWatcher;
// number of items currently in loading (populating) state
static int mPopulatingCount;
Expand Down
2 changes: 2 additions & 0 deletions src/providers/postgres/qgspostgresdataitems.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ QVector<QgsDataItem*> QgsPGConnectionItem::createChildren()
QVector<QgsDataItem*>items;

QgsDataSourceURI uri = QgsPostgresConn::connUri( mName );
// TODO: wee need to cancel somehow acquireConnection() if deleteLater() was called on this item to avoid later credential dialog if connection failed
QgsPostgresConn *conn = QgsPostgresConnPool::instance()->acquireConnection( uri.connectionInfo() );
if ( !conn )
{
Expand Down Expand Up @@ -302,6 +303,7 @@ QVector<QgsDataItem*> QgsPGSchemaItem::createChildren()
QgsPostgresConn::geometryColumnsOnly( mName ),
QgsPostgresConn::publicSchemaOnly( mName ),
QgsPostgresConn::allowGeometrylessTables( mName ) );

if ( !ok )
{
items.append( new QgsErrorItem( this, tr( "Failed to get layers" ), mPath + "/error" ) );
Expand Down

0 comments on commit 138d836

Please sign in to comment.