Fix possible memleak in CDatabaseWriter

* CApplication network methods cleanup

src/blackcore/airportdatareader.cpp

@@ -61,7 +61,8 @@ namespace BlackCore
             return;
         }
 
-        CAirportList airports = CAirportList::fromDatabaseJson(array);
+        CAirportList airports;
+        airports.convertFromDatabaseJson(array);
         quint64 timestamp = lastModifiedMsSinceEpoch(nwReply);
 
         {

src/blackcore/airportdatareader.h

@@ -61,7 +61,7 @@ namespace BlackCore
         void ps_airportCacheChanged();
 
     private:
-        BlackMisc::CData<BlackCore::Data::DbAirportCache> m_airportCache {this, &CAirportDataReader::ps_airportCacheChanged};
+        BlackMisc::CData<BlackCore::Data::TDbAirportCache> m_airportCache {this, &CAirportDataReader::ps_airportCacheChanged};
         mutable QReadWriteLock m_lock;
         quint64 m_lastModified = 0; //!< When was data file updated, obtained from HTTP Last-Modified header, in ms from epoch
 

src/blackcore/application.cpp

@@ -398,106 +398,44 @@ namespace BlackCore
 
     QNetworkReply *CApplication::getFromNetwork(const CUrl &url, const CSlot<void(QNetworkReply *)> &callback)
     {
-        if (this->m_shutdown) { return nullptr; }
+        return httpRequestImpl(url.toNetworkRequest(), callback, [ ] (QNetworkAccessManager &nam, const QNetworkRequest &request) { return nam.get(request); });
-        return getFromNetwork(url.toNetworkRequest(), callback);
     }
 
     QNetworkReply *CApplication::getFromNetwork(const QNetworkRequest &request, const CSlot<void(QNetworkReply *)> &callback)
     {
-        if (this->m_shutdown) { return nullptr; }
+        return httpRequestImpl(request, callback, [ ] (QNetworkAccessManager &nam, const QNetworkRequest &request) { return nam.get(request); });
-        QWriteLocker locker(&m_accessManagerLock);
-        Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread");
-        if (QThread::currentThread() != this->m_accessManager.thread())
-        {
-            QTimer::singleShot(0, this, [this, request, callback]() { this->getFromNetwork(request, callback); });
-            return nullptr; // not yet started
-        }
-
-        Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch");
-        QNetworkRequest r(request); // no QObject
-        CNetworkUtils::ignoreSslVerification(r);
-        CNetworkUtils::setSwiftUserAgent(r);
-        QNetworkReply *reply = this->m_accessManager.get(r);
-        if (callback)
-        {
-            connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }, Qt::QueuedConnection);
-        }
-        return reply;
     }
 
     QNetworkReply *CApplication::postToNetwork(const QNetworkRequest &request, const QByteArray &data, const CSlot<void(QNetworkReply *)> &callback)
     {
-        if (this->m_shutdown) { return nullptr; }
+        return httpRequestImpl(request, callback, [ data ] (QNetworkAccessManager &nam, const QNetworkRequest &request) { return nam.post(request, data); });
-        QWriteLocker locker(&m_accessManagerLock);
-        Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread");
-        if (QThread::currentThread() != this->m_accessManager.thread())
-        {
-            QTimer::singleShot(0, this, [this, request, data, callback]() { this->postToNetwork(request, data, callback); });
-            return nullptr; // not yet started
-        }
-
-        Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch");
-        QNetworkRequest r(request);
-        CNetworkUtils::ignoreSslVerification(r);
-        CNetworkUtils::setSwiftUserAgent(r);
-        QNetworkReply *reply = this->m_accessManager.post(r, data);
-        if (callback)
-        {
-            connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }, Qt::QueuedConnection);
-        }
-        return reply;
     }
 
     QNetworkReply *CApplication::postToNetwork(const QNetworkRequest &request, QHttpMultiPart *multiPart, const CSlot<void(QNetworkReply *)> &callback)
     {
-        if (this->m_shutdown) { return nullptr; }
-        QWriteLocker locker(&m_accessManagerLock);
-        Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread");
         if (QThread::currentThread() != this->m_accessManager.thread())
         {
-            QTimer::singleShot(0, this, [this, request, multiPart, callback]() { this->postToNetwork(request, multiPart, callback); });
+            multiPart->moveToThread(this->m_accessManager.thread());
-            return nullptr; // not yet started
         }
 
-        Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch");
+        return httpRequestImpl(request, callback, [ this, multiPart ] (QNetworkAccessManager &nam, const QNetworkRequest &request)
-        QNetworkRequest r(request);
+            {
-        CNetworkUtils::ignoreSslVerification(r);
+                QNetworkReply *reply = nam.post(request, multiPart);
-        CNetworkUtils::setSwiftUserAgent(r);
+                Q_ASSERT(reply);
-        QNetworkReply *reply = this->m_accessManager.post(r, multiPart);
+                multiPart->setParent(reply);
-        if (callback)
+                return reply;
-        {
+            }
-            connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); });
+        );
-        }
-        return reply;
     }
 
     QNetworkReply *CApplication::headerFromNetwork(const CUrl &url, const BlackMisc::CSlot<void (QNetworkReply *)> &callback)
     {
-        if (this->m_shutdown) { return nullptr; }
+        return httpRequestImpl(url.toNetworkRequest(), callback, [ ] (QNetworkAccessManager &nam, const QNetworkRequest &request) { return nam.head(request); });
-        return headerFromNetwork(url.toNetworkRequest(), callback);
     }
 
     QNetworkReply *CApplication::headerFromNetwork(const QNetworkRequest &request, const BlackMisc::CSlot<void (QNetworkReply *)> &callback)
     {
-        if (this->m_shutdown) { return nullptr; }
+        return httpRequestImpl(request, callback, [ ] (QNetworkAccessManager &nam, const QNetworkRequest &request) { return nam.head(request); });
-        QWriteLocker locker(&m_accessManagerLock);
-        Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread");
-        if (QThread::currentThread() != this->m_accessManager.thread())
-        {
-            QTimer::singleShot(0, this, [this, request, callback]() { this->headerFromNetwork(request, callback); });
-            return nullptr; // not yet started
-        }
-
-        Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch");
-        QNetworkRequest r(request); // no QObject
-        CNetworkUtils::ignoreSslVerification(r);
-        CNetworkUtils::setSwiftUserAgent(r);
-        QNetworkReply *reply = this->m_accessManager.head(r);
-        if (callback)
-        {
-            connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }, Qt::QueuedConnection);
-        }
-        return reply;
     }
 
     void CApplication::deleteAllCookies()
@@ -1060,4 +998,27 @@ namespace BlackCore
         #endif
     }
 
+    QNetworkReply *CApplication::httpRequestImpl(const QNetworkRequest &request, const BlackMisc::CSlot<void (QNetworkReply *)> &callback, std::function<QNetworkReply *(QNetworkAccessManager &, const QNetworkRequest &)> method)
+    {
+        if (this->m_shutdown) { return nullptr; }
+        QWriteLocker locker(&m_accessManagerLock);
+        Q_ASSERT_X(QCoreApplication::instance()->thread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager supposed to be in main thread");
+        if (QThread::currentThread() != this->m_accessManager.thread())
+        {
+//            QTimer::singleShot(0, this, [this, request, callback, method]() { this->httpRequestImpl(request, callback, method); });
+            QTimer::singleShot(0, this, std::bind(&CApplication::httpRequestImpl, this, request, callback, method));
+            return nullptr; // not yet started
+        }
+
+        Q_ASSERT_X(QThread::currentThread() == m_accessManager.thread(), Q_FUNC_INFO, "Network manager thread mismatch");
+        QNetworkRequest r(request); // no QObject
+        CNetworkUtils::ignoreSslVerification(r);
+        CNetworkUtils::setSwiftUserAgent(r);
+        QNetworkReply *reply = method(this->m_accessManager, r);
+        if (callback)
+        {
+            connect(reply, &QNetworkReply::finished, callback.object(), [ = ] { callback(reply); }, Qt::QueuedConnection);
+        }
+        return reply;
+    }
 } // ns

src/blackcore/application.h

@@ -23,6 +23,7 @@
 #include <QString>
 #include <QStringList>
 #include <atomic>
+#include <functional>
 
 #include "blackcore/blackcoreexport.h"
 #include "blackcore/cookiemanager.h"
@@ -321,6 +322,7 @@ namespace BlackCore
                                      const BlackMisc::CSlot<void(QNetworkReply *)> &callback);
 
         //! Post to network
+        //! \note This method takes ownership over \c multiPart.
         //! \threadsafe
         QNetworkReply *postToNetwork(const QNetworkRequest &request, QHttpMultiPart *multiPart,
                                      const BlackMisc::CSlot<void(QNetworkReply *)> &callback);
@@ -427,6 +429,11 @@ namespace BlackCore
 
         void crashDumpUploadEnabledChanged();
 
+        //! Implementation for getFromNetwork(), postToNetwork() and headerFromNetwork()
+        QNetworkReply *httpRequestImpl(const QNetworkRequest &request,
+                                       const BlackMisc::CSlot<void(QNetworkReply *)> &callback,
+                                       std::function<QNetworkReply *(QNetworkAccessManager &, const QNetworkRequest &)> method);
+
         QScopedPointer<CCoreFacade>              m_coreFacade;             //!< core facade if any
         QScopedPointer<CSetupReader>             m_setupReader;            //!< setup reader
         QScopedPointer<CWebDataServices>         m_webDataServices;        //!< web data services

src/blackcore/data/dbcaches.h

@@ -111,7 +111,7 @@ namespace BlackCore
         };
 
         //! Trait for airport list
-        struct DbAirportCache : public BlackMisc::CDataTrait<BlackMisc::Aviation::CAirportList>
+        struct TDbAirportCache : public BlackMisc::TDataTrait<BlackMisc::Aviation::CAirportList>
         {
             //! Defer loading
             static constexpr bool isDeferred() { return true; }

src/blackcore/db/databasewriter.cpp

@@ -66,7 +66,6 @@ namespace BlackCore
             }
 
             m_pendingReply = sApp->postToNetwork(request, multiPart, { this, &CDatabaseWriter::ps_postResponse});
-            multiPart->setParent(m_pendingReply);
             return msgs;
         }
 

src/blackcore/webreaderflags.h

@@ -39,8 +39,8 @@ namespace BlackCore
             AirportReader        = 1 << 6,   //!< reader for airport list
             InfoDataReader       = 1 << 7,   //!< DB info data (metdata, how many data, when updated)
             AllVatsimReaders     = VatsimBookingReader | VatsimDataReader | VatsimMetarReader | VatsimStatusReader,  //!< all VATSIM readers
-            AllSwiftDbReaders    = IcaoDataReader | ModelReader | AirportReader | InfoDataReader,                    //!< all swift data
+            AllSwiftDbReaders    = IcaoDataReader | ModelReader | InfoDataReader,                                    //!< all swift data
-            AllReaders           = AllSwiftDbReaders | AllVatsimReaders                                              //!< everything
+            AllReaders           = AirportReader | AllSwiftDbReaders | AllVatsimReaders                              //!< everything
         };
         Q_DECLARE_FLAGS(WebReader, WebReaderFlag)
 

src/blackmisc/aviation/airport.cpp

@@ -52,13 +52,16 @@ namespace BlackMisc
             (void)QT_TRANSLATE_NOOP("Aviation", "Airport");
         }
 
-        void CAirport::fromDatabaseJson(const QJsonObject &json)
+        void CAirport::convertFromDatabaseJson(const QJsonObject &json)
         {
             Q_ASSERT(json.value("icao").isString());
             setIcao(json.value("icao").toString());
 
-            Q_ASSERT(json.value("country").isString());
+            if (json.value("alpha3").isString() && json.value("country").isString())
-            setCountry(json.value("country").toString());
+            {
+                CCountry country(json.value("alpha3").toString(), json.value("country").toString());
+                setCountry(country);
+            }
 
             Q_ASSERT(json.value("name").isString());
             setDescriptiveName(json.value("name").toString());

src/blackmisc/aviation/airport.h

@@ -19,6 +19,7 @@
 #include "blackmisc/geo/longitude.h"
 #include "blackmisc/metaclass.h"
 #include "blackmisc/pq/length.h"
+#include "blackmisc/country.h"
 #include "blackmisc/propertyindex.h"
 #include "blackmisc/valueobject.h"
 #include "blackmisc/variant.h"
@@ -80,10 +81,10 @@ namespace BlackMisc
             void setPosition(const BlackMisc::Geo::CCoordinateGeodetic &position) { this->m_position = position; }
 
             //! Get the country
-            QString getCountry() const { return m_country; }
+            const CCountry& getCountry() const { return m_country; }
 
             //! Set the country
-            void setCountry(const QString &country) { this->m_country = country; }
+            void setCountry(const CCountry &country) { this->m_country = country; }
 
             //! Elevation
             //! \sa geodeticHeight
@@ -131,13 +132,13 @@ namespace BlackMisc
             QString convertToQString(bool i18n = false) const;
 
             //! \copydoc BlackMisc::CValueObject::convertFromJson
-            void fromDatabaseJson(const QJsonObject &json);
+            void convertFromDatabaseJson(const QJsonObject &json);
 
         private:
             CAirportIcaoCode                    m_icao;
             QString                             m_descriptiveName;
             BlackMisc::Geo::CCoordinateGeodetic m_position;
-            QString                             m_country;
+            CCountry                            m_country;
 
             BLACK_METACLASS(
                 CAirport,

src/blackmisc/aviation/airportlist.cpp

@@ -44,18 +44,17 @@ namespace BlackMisc
             return this->findFirstByOrDefault(&CAirport::getIcao, icao, ifNotFound);
         }
 
-        CAirportList CAirportList::fromDatabaseJson(const QJsonArray &json)
+        void CAirportList::convertFromDatabaseJson(const QJsonArray &json)
         {
-            CAirportList airports;
+            clear();
+
             for (const QJsonValue& value: json)
             {
                 QJsonObject object = value.toObject();
                 CAirport airport;
-                airport.fromDatabaseJson(object);
+                airport.convertFromDatabaseJson(object);
-                airports.push_back(airport);
+                push_back(airport);
             }
-
-            return airports;
         }
 
     } // namespace

src/blackmisc/aviation/airportlist.h

@@ -50,8 +50,9 @@ namespace BlackMisc
             //! Find first station by callsign, if not return given value / default
             CAirport findFirstByIcao(const CAirportIcaoCode &icao, const CAirport &ifNotFound = CAirport()) const;
 
-            //! Reads the airport list from JSON
+            //! Reads the airport list from database JSON
-            static CAirportList fromDatabaseJson(const QJsonArray& json);
+            void convertFromDatabaseJson(const QJsonArray& json);
+
         };
     } //namespace
 } // namespace