Fix STAC memory leaks

src/core/stac/qgsstaccontroller.cpp

@@ -17,6 +17,7 @@
 #include "moc_qgsstaccontroller.cpp"
 #include "qgsstaccatalog.h"
 #include "qgsstaccollection.h"
+#include "qgsstaccollections.h"
 #include "qgsstacitem.h"
 #include "qgsstacitemcollection.h"
 #include "qgsstacparser.h"
@@ -32,9 +33,11 @@
 QgsStacController::~QgsStacController()
 {
   qDeleteAll( mReplies );
+  qDeleteAll( mFetchedStacObjects );
+  qDeleteAll( mFetchedItemCollections );
+  qDeleteAll( mFetchedCollections );
 }
 
-
 int QgsStacController::fetchStacObjectAsync( const QUrl &url )
 {
   QNetworkReply *reply = fetchAsync( url );
@@ -211,22 +214,25 @@ void QgsStacController::handleCollectionsReply()
   mReplies.removeOne( reply );
 }
 
-QgsStacObject *QgsStacController::takeStacObject( int requestId )
+std::unique_ptr< QgsStacObject > QgsStacController::takeStacObject( int requestId )
 {
-  return mFetchedStacObjects.take( requestId );
+  std::unique_ptr< QgsStacObject > obj( mFetchedStacObjects.take( requestId ) );
+  return obj;
 }
 
-QgsStacItemCollection *QgsStacController::takeItemCollection( int requestId )
+std::unique_ptr< QgsStacItemCollection > QgsStacController::takeItemCollection( int requestId )
 {
-  return mFetchedItemCollections.take( requestId );
+  std::unique_ptr< QgsStacItemCollection > col( mFetchedItemCollections.take( requestId ) );
+  return col;
 }
 
-QgsStacCollections *QgsStacController::takeCollections( int requestId )
+std::unique_ptr< QgsStacCollections > QgsStacController::takeCollections( int requestId )
 {
-  return mFetchedCollections.take( requestId );
+  std::unique_ptr< QgsStacCollections > cols( mFetchedCollections.take( requestId ) );
+  return cols;
 }
 
-QgsStacObject *QgsStacController::fetchStacObject( const QUrl &url, QString *error )
+std::unique_ptr< QgsStacObject > QgsStacController::fetchStacObject( const QUrl &url, QString *error )
 {
   QgsNetworkReplyContent content = fetchBlocking( url );
 
@@ -243,20 +249,19 @@ QgsStacObject *QgsStacController::fetchStacObject( const QUrl &url, QString *err
   QgsStacParser parser;
   parser.setData( data );
   parser.setBaseUrl( url );
-  QgsStacObject *object = nullptr;
+  std::unique_ptr< QgsStacObject > object;
   switch ( parser.type() )
   {
     case QgsStacObject::Type::Catalog:
-      object = parser.catalog();
+      object.reset( parser.catalog() );
       break;
     case QgsStacObject::Type::Collection:
-      object = parser.collection();
+      object.reset( parser.collection() );
       break;
     case QgsStacObject::Type::Item:
-      object = parser.item();
+      object.reset( parser.item() );
       break;
     case QgsStacObject::Type::Unknown:
-      object = nullptr;
       break;
   }
 
@@ -266,7 +271,7 @@ QgsStacObject *QgsStacController::fetchStacObject( const QUrl &url, QString *err
   return object;
 }
 
-QgsStacItemCollection *QgsStacController::fetchItemCollection( const QUrl &url, QString *error )
+std::unique_ptr< QgsStacItemCollection > QgsStacController::fetchItemCollection( const QUrl &url, QString *error )
 {
   QgsNetworkReplyContent content = fetchBlocking( url );
 
@@ -283,15 +288,15 @@ QgsStacItemCollection *QgsStacController::fetchItemCollection( const QUrl &url,
   QgsStacParser parser;
   parser.setData( data );
   parser.setBaseUrl( url );
-  QgsStacItemCollection *ic = parser.itemCollection();
+  std::unique_ptr< QgsStacItemCollection > ic( parser.itemCollection() );
 
   if ( error )
     *error = parser.error();
 
   return ic;
 }
 
-QgsStacCollections *QgsStacController::fetchCollections( const QUrl &url, QString *error )
+std::unique_ptr< QgsStacCollections > QgsStacController::fetchCollections( const QUrl &url, QString *error )
 {
   QgsNetworkReplyContent content = fetchBlocking( url );
 
@@ -307,7 +312,7 @@ QgsStacCollections *QgsStacController::fetchCollections( const QUrl &url, QStrin
 
   QgsStacParser parser;
   parser.setData( data );
-  QgsStacCollections *col = parser.collections();
+  std::unique_ptr< QgsStacCollections > col( parser.collections() );
 
   if ( error )
     *error = parser.error();
@@ -391,8 +396,3 @@ QgsStacItem *QgsStacController::openLocalItem( const QString &fileName ) const
   parser.setBaseUrl( fileName );
   return parser.item();
 }
-
-
-
-
-

src/core/stac/qgsstaccontroller.h

@@ -75,21 +75,21 @@ class CORE_EXPORT QgsStacController : public QObject
      * An optional \a error parameter will be populated with any network error information.
      * The caller takes ownership of the returned object
      */
-    QgsStacObject *fetchStacObject( const QUrl &url, QString *error = nullptr );
+    std::unique_ptr< QgsStacObject > fetchStacObject( const QUrl &url, QString *error = nullptr );
 
     /**
      * Fetches a feature collection from \a url using a blocking network request.
      * An optional \a error parameter will be populated with any network error information.
      * The caller takes ownership of the returned feature collection
      */
-    QgsStacItemCollection *fetchItemCollection( const QUrl &url, QString *error = nullptr );
+    std::unique_ptr< QgsStacItemCollection > fetchItemCollection( const QUrl &url, QString *error = nullptr );
 
     /**
      * Fetches collections from \a url using a blocking network request.
      * An optional \a error parameter will be populated with any network error information.
      * The caller takes ownership of the returned feature collection
      */
-    QgsStacCollections *fetchCollections( const QUrl &url, QString *error = nullptr );
+    std::unique_ptr< QgsStacCollections > fetchCollections( const QUrl &url, QString *error = nullptr );
 
     /**
      * Initiates an asynchronous request for a STAC object using the \a url
@@ -130,7 +130,7 @@ class CORE_EXPORT QgsStacController : public QObject
      * \see fetchStacObjectAsync
      * \see finishedStacObjectRequest
      */
-    QgsStacObject *takeStacObject( int requestId );
+    std::unique_ptr< QgsStacObject > takeStacObject( int requestId );
 
     /**
      * Returns the feature collection fetched with the specified \a requestId
@@ -140,7 +140,7 @@ class CORE_EXPORT QgsStacController : public QObject
      * \see fetchItemCollectionAsync
      * \see finishedItemCollectionRequest
      */
-    QgsStacItemCollection *takeItemCollection( int requestId );
+    std::unique_ptr< QgsStacItemCollection > takeItemCollection( int requestId );
 
     /**
      * Returns the collections collection fetched with the specified \a requestId
@@ -151,7 +151,7 @@ class CORE_EXPORT QgsStacController : public QObject
      * \see finishedCollectionsRequest
      * \since QGIS 3.42
      */
-    QgsStacCollections *takeCollections( int requestId );
+    std::unique_ptr< QgsStacCollections > takeCollections( int requestId );
 
     /**
      * Returns the authentication config id which will be used during the request.

src/core/stac/qgsstacdataitems.cpp

@@ -69,9 +69,8 @@ QVector<QgsDataItem *> QgsStacItemItem::createChildren()
 {
   QgsStacController *controller = stacController();
   QString error;
-  QgsStacObject *obj = controller->fetchStacObject( mPath, &error );
-  QgsStacItem *item = dynamic_cast<QgsStacItem *>( obj );
-  setStacItem( item );
+  std::unique_ptr< QgsStacObject > obj = controller->fetchStacObject( mPath, &error );
+  setStacItem( obj );
 
   if ( !mStacItem )
     return { new QgsErrorItem( this, error, path() + QStringLiteral( "/error" ) ) };
@@ -175,8 +174,15 @@ QgsStacController *QgsStacItemItem::stacController()
   return nullptr;
 }
 
-void QgsStacItemItem::setStacItem( QgsStacItem *item )
+void QgsStacItemItem::setStacItem( std::unique_ptr< QgsStacObject > &object )
 {
+  QgsStacItem *item = dynamic_cast<QgsStacItem *>( object.get() );
+  if ( item )
+  {
+    // release object, mStacItem will take ownership of the successfully cast item
+    ( void )object.release();
+  }
+
   mStacItem.reset( item );
   updateToolTip();
 }
@@ -189,15 +195,14 @@ QgsStacItem *QgsStacItemItem::stacItem() const
 void QgsStacItemItem::itemRequestFinished( int requestId, QString error )
 {
   QgsStacController *controller = stacController();
-  QgsStacObject *object = controller->takeStacObject( requestId );
-  QgsStacItem *item = dynamic_cast< QgsStacItem * >( object );
-  setStacItem( item );
-  if ( item )
+  std::unique_ptr< QgsStacObject > object = controller->takeStacObject( requestId );
+  setStacItem( object );
+  if ( mStacItem )
   {
     mIconName = QStringLiteral( "mActionPropertiesWidget.svg" );
-    QString name = item->properties().value( QStringLiteral( "title" ), QString() ).toString();
+    QString name = mStacItem->properties().value( QStringLiteral( "title" ), QString() ).toString();
     if ( name.isEmpty() )
-      name = item->id();
+      name = mStacItem->id();
     mName = name;
   }
   else
@@ -297,7 +302,7 @@ void QgsStacCatalogItem::childrenCreated()
 
 void QgsStacCatalogItem::onControllerFinished( int requestId, const QString &error )
 {
-  for ( auto child : std::as_const( mChildren ) )
+  for ( QgsDataItem *child : std::as_const( mChildren ) )
   {
     if ( child->state() != Qgis::BrowserItemState::NotPopulated )
       continue;
@@ -317,9 +322,8 @@ QVector<QgsDataItem *> QgsStacCatalogItem::createChildren()
 
   QgsStacController *controller = stacController();
   QString error;
-  QgsStacObject *obj = controller->fetchStacObject( mPath, &error );
-  QgsStacCatalog *cat = dynamic_cast<QgsStacCatalog *>( obj );
-  setStacCatalog( cat );
+  std::unique_ptr< QgsStacObject > obj = controller->fetchStacObject( mPath, &error );
+  setStacCatalog( obj );
 
   if ( !mStacCatalog )
     return { new QgsErrorItem( this, error, path() + QStringLiteral( "/error" ) ) };
@@ -338,7 +342,7 @@ QVector<QgsDataItem *> QgsStacCatalogItem::createChildren()
   bool useCollectionsEndpoint = false;
   if ( supportsCollections || supportsItems )
   {
-    for ( const auto &link : links )
+    for ( const QgsStacLink &link : links )
     {
       if ( link.relation() == QLatin1String( "items" ) )
       {
@@ -355,7 +359,7 @@ QVector<QgsDataItem *> QgsStacCatalogItem::createChildren()
     }
   }
 
-  for ( const auto &link : links )
+  for ( const QgsStacLink &link : links )
   {
     // skip hierarchical navigation links
     if ( link.relation() == QLatin1String( "self" ) ||
@@ -458,8 +462,15 @@ void QgsStacCatalogItem::updateToolTip()
   }
 }
 
-void QgsStacCatalogItem::setStacCatalog( QgsStacCatalog *catalog )
+void QgsStacCatalogItem::setStacCatalog( std::unique_ptr< QgsStacObject > &object )
 {
+  QgsStacCatalog *catalog = dynamic_cast<QgsStacCatalog *>( object.get() );
+  if ( catalog )
+  {
+    // release object, mStacCatalog will take ownership of the successfully cast catalog
+    ( void )object.release();
+  }
+
   mStacCatalog.reset( catalog );
   if ( mStacCatalog )
   {
@@ -489,10 +500,12 @@ QVector< QgsDataItem * > QgsStacCatalogItem::createItems( const QVector<QgsStacI
     if ( !item )
       continue;
 
+    std::unique_ptr< QgsStacObject > object( item );
+
     const QString name = item->properties().value( QStringLiteral( "title" ), item->id() ).toString();
 
     QgsStacItemItem *i = new QgsStacItemItem( this, name, item->url() );
-    i->setStacItem( item );
+    i->setStacItem( object );
     i->setState( Qgis::BrowserItemState::Populated );
     contents.append( i );
   }
@@ -508,10 +521,12 @@ QVector<QgsDataItem *> QgsStacCatalogItem::createCollections( const QVector<QgsS
     if ( !col )
       continue;
 
+    std::unique_ptr< QgsStacObject > object( col );
+
     const QString name = col->title().isEmpty() ? col->id() : col->title();
 
     QgsStacCatalogItem *i = new QgsStacCatalogItem( this, name, col->url() );
-    i->setStacCatalog( col );
+    i->setStacCatalog( object );
     contents.append( i );
   }
   return contents;
@@ -591,7 +606,7 @@ QgsStacRootItem::QgsStacRootItem( QgsDataItem *parent, const QString &name, cons
 QVector<QgsDataItem *> QgsStacRootItem::createChildren()
 {
   QVector<QgsDataItem *> connections;
-  const auto connectionList = QgsStacConnection::connectionList();
+  const QStringList connectionList = QgsStacConnection::connectionList();
   for ( const QString &connName : connectionList )
   {
     QgsDataItem *conn = new QgsStacConnectionItem( this, connName );

src/core/stac/qgsstacdataitems.h

@@ -59,7 +59,7 @@ class CORE_EXPORT QgsStacItemItem : public QgsDataItem
     QgsStacController *stacController();
 
     //! takes ownership
-    void setStacItem( QgsStacItem *item );
+    void setStacItem( std::unique_ptr< QgsStacObject > &object );
 
     //! does not transfer ownership
     QgsStacItem *stacItem() const;
@@ -87,7 +87,7 @@ class CORE_EXPORT QgsStacCatalogItem : public QgsDataCollectionItem
     void updateToolTip();
 
     //! takes ownership
-    void setStacCatalog( QgsStacCatalog *catalog );
+    void setStacCatalog( std::unique_ptr< QgsStacObject > &object );
 
     //! does not transfer ownership
     QgsStacCatalog *stacCatalog() const;