HG_REPOSITORIES/LPP/SciQLOP_Repos/SciQLop Commit - r423:3698da027722

Add thread protection on AbortDownload process

perrinel -

r423:3698da027722

parent child

Context file:

r423:3698da027722

core/src/Network/NetworkController.cpp +37 -4

             #include "Network/NetworkController.h"
             #include <QMutex>
             #include <QNetworkAccessManager>
             #include <QNetworkReply>
             #include <QNetworkRequest>
+            #include <QReadWriteLock>
             #include <QThread>
             #include <unordered_map>
             Q_LOGGING_CATEGORY(LOG_NetworkController, "NetworkController")
             struct NetworkController::NetworkControllerPrivate {
                 explicit NetworkControllerPrivate(NetworkController *parent) : m_WorkingMutex{} {}
                 QMutex m_WorkingMutex;
+                QReadWriteLock m_Lock;
                 std::unordered_map<QNetworkReply *, QUuid> m_NetworkReplyToVariableId;
                 std::unique_ptr<QNetworkAccessManager> m_AccessManager{nullptr};
+                void lockRead() { m_Lock.lockForRead(); }
+                void lockWrite() { m_Lock.lockForWrite(); }
+                void unlock() { m_Lock.unlock(); }
             };
             NetworkController::NetworkController(QObject *parent)
                     : QObject(parent), impl{spimpl::make_unique_impl<NetworkControllerPrivate>(this)}
             {
             }
             void NetworkController::onProcessRequested(const QNetworkRequest &request, QUuid identifier,
                                                        std::function<void(QNetworkReply *, QUuid)> callback)
             {
-                qCInfo(LOG_NetworkController()) << tr("NetworkController registered")
-                                                << QThread::currentThread();
                 auto reply = impl->m_AccessManager->get(request);
+                qCDebug(LOG_NetworkController()) << tr("NetworkController registered")
+                                                 << QThread::currentThread() << reply;
                 // Store the couple reply id
+                impl->lockWrite();
                 impl->m_NetworkReplyToVariableId[reply] = identifier;
+                impl->unlock();
                 auto onReplyFinished = [reply, this, identifier, callback]() {
-                    qCInfo(LOG_NetworkController()) << tr("NetworkController onReplyFinished")
+                    qCDebug(LOG_NetworkController()) << tr("NetworkController onReplyFinished")
-                                                    << QThread::currentThread();
+                                                     << QThread::currentThread() << reply;
+                    impl->lockRead();
                     auto it = impl->m_NetworkReplyToVariableId.find(reply);
+                    impl->unlock();
                     if (it != impl->m_NetworkReplyToVariableId.cend()) {
+                        impl->lockWrite();
+                        impl->m_NetworkReplyToVariableId.erase(reply);
+                        impl->unlock();
+                        // Deletes reply
                         callback(reply, identifier);
+                        reply->deleteLater();
+                        emit this->replyDownloadProgress(identifier, 0);
                     }
+                    qCDebug(LOG_NetworkController()) << tr("NetworkController onReplyFinished END")
+                                                     << QThread::currentThread() << reply;
                 };
                 auto onReplyProgress = [reply, this](qint64 bytesRead, qint64 totalBytes) {
                     double progress = (bytesRead * 100.0) / totalBytes;
+                    qCDebug(LOG_NetworkController()) << tr("NetworkController onReplyProgress") << progress
+                                                     << QThread::currentThread() << reply;
+                    impl->lockRead();
                     auto it = impl->m_NetworkReplyToVariableId.find(reply);
+                    impl->unlock();
                     if (it != impl->m_NetworkReplyToVariableId.cend()) {
                         emit this->replyDownloadProgress(it->second, progress);
                     }
+                    qCDebug(LOG_NetworkController()) << tr("NetworkController onReplyProgress END")
+                                                     << QThread::currentThread() << reply;
                 };
                 connect(reply, &QNetworkReply::finished, this, onReplyFinished);
                 connect(reply, &QNetworkReply::downloadProgress, this, onReplyProgress);
             }
             void NetworkController::initialize()
             {
                 qCDebug(LOG_NetworkController()) << tr("NetworkController init") << QThread::currentThread();
                 impl->m_WorkingMutex.lock();
                 impl->m_AccessManager = std::make_unique<QNetworkAccessManager>();
                 qCDebug(LOG_NetworkController()) << tr("NetworkController init END");
             }
             void NetworkController::finalize()
             {
                 impl->m_WorkingMutex.unlock();
             }
             void NetworkController::onReplyCanceled(QUuid identifier)
             {
                 auto findReply = [identifier](const auto &entry) { return identifier == entry.second; };
+                qCDebug(LOG_NetworkController()) << tr("NetworkController onReplyCanceled")
+                                                 << QThread::currentThread();
+                impl->lockRead();
                 auto end = impl->m_NetworkReplyToVariableId.cend();
                 auto it = std::find_if(impl->m_NetworkReplyToVariableId.cbegin(), end, findReply);
+                impl->unlock();
                 if (it != end) {
                     it->first->abort();
                 }
+                qCDebug(LOG_NetworkController()) << tr("NetworkController onReplyCanceled END")
+                                                 << QThread::currentThread();
             }
             void NetworkController::waitForFinish()
             {
                 QMutexLocker locker{&impl->m_WorkingMutex};
             }

core/src/Variable/VariableController.cpp +2 -2

             #include <Variable/Variable.h>
             #include <Variable/VariableCacheController.h>
             #include <Variable/VariableController.h>
             #include <Variable/VariableModel.h>
             #include <Data/DataProviderParameters.h>
             #include <Data/IDataProvider.h>
             #include <Data/IDataSeries.h>
             #include <Time/TimeController.h>
             #include <QDateTime>
             #include <QMutex>
             #include <QThread>
             #include <QUuid>
             #include <QtCore/QItemSelectionModel>
             #include <unordered_map>
             Q_LOGGING_CATEGORY(LOG_VariableController, "VariableController")
             struct VariableController::VariableControllerPrivate {
                 explicit VariableControllerPrivate(VariableController *parent)
                         : m_WorkingMutex{},
                           m_VariableModel{new VariableModel{parent}},
                           m_VariableSelectionModel{new QItemSelectionModel{m_VariableModel, parent}},
                           m_VariableCacheController{std::make_unique<VariableCacheController>()}
                 {
                 }
                 QMutex m_WorkingMutex;
                 /// Variable model. The VariableController has the ownership
                 VariableModel *m_VariableModel;
                 QItemSelectionModel *m_VariableSelectionModel;
                 TimeController *m_TimeController{nullptr};
                 std::unique_ptr<VariableCacheController> m_VariableCacheController;
                 std::unordered_map<std::shared_ptr<Variable>, std::shared_ptr<IDataProvider> >
                     m_VariableToProviderMap;
                 std::unordered_map<std::shared_ptr<Variable>, QUuid> m_VariableToIdentifierMap;
             };
             VariableController::VariableController(QObject *parent)
                     : QObject{parent}, impl{spimpl::make_unique_impl<VariableControllerPrivate>(this)}
             {
                 qCDebug(LOG_VariableController()) << tr("VariableController construction")
                                                   << QThread::currentThread();
                 connect(impl->m_VariableModel, &VariableModel::abortProgessRequested, this,
                         &VariableController::onAbortProgressRequested);
             }
             VariableController::~VariableController()
             {
                 qCDebug(LOG_VariableController()) << tr("VariableController destruction")
                                                   << QThread::currentThread();
                 this->waitForFinish();
             }
             VariableModel *VariableController::variableModel() noexcept
             {
                 return impl->m_VariableModel;
             }
             QItemSelectionModel *VariableController::variableSelectionModel() noexcept
             {
                 return impl->m_VariableSelectionModel;
             }
             void VariableController::setTimeController(TimeController *timeController) noexcept
             {
                 impl->m_TimeController = timeController;
             }
             void VariableController::deleteVariable(std::shared_ptr<Variable> variable) noexcept
             {
                 if (!variable) {
                     qCCritical(LOG_VariableController()) << "Can't delete variable: variable is null";
                     return;
                 }
                 // Spreads in SciQlop that the variable will be deleted, so that potential receivers can
                 // make some treatments before the deletion
                 emit variableAboutToBeDeleted(variable);
                 // Deletes identifier
                 impl->m_VariableToIdentifierMap.erase(variable);
                 // Deletes provider
                 auto nbProvidersDeleted = impl->m_VariableToProviderMap.erase(variable);
                 qCDebug(LOG_VariableController())
                     << tr("Number of providers deleted for variable %1: %2")
                            .arg(variable->name(), QString::number(nbProvidersDeleted));
                 // Clears cache
                 impl->m_VariableCacheController->clear(variable);
                 // Deletes from model
                 impl->m_VariableModel->deleteVariable(variable);
             }
             void VariableController::deleteVariables(
                 const QVector<std::shared_ptr<Variable> > &variables) noexcept
             {
                 for (auto variable : qAsConst(variables)) {
                     deleteVariable(variable);
                 }
             }
             void VariableController::abortProgress(std::shared_ptr<Variable> variable)
             {
             }
             void VariableController::createVariable(const QString &name, const QVariantHash &metadata,
                                                     std::shared_ptr<IDataProvider> provider) noexcept
             {
                 if (!impl->m_TimeController) {
                     qCCritical(LOG_VariableController())
                         << tr("Impossible to create variable: The time controller is null");
                     return;
                 }
                 auto dateTime = impl->m_TimeController->dateTime();
                 if (auto newVariable = impl->m_VariableModel->createVariable(name, dateTime, metadata)) {
                     auto identifier = QUuid::createUuid();
                     // store the provider
                     impl->m_VariableToProviderMap[newVariable] = provider;
                     impl->m_VariableToIdentifierMap[newVariable] = identifier;
                     auto addDateTimeAcquired = [ this, varW = std::weak_ptr<Variable>{newVariable} ](
                         QUuid identifier, auto dataSeriesAcquired, auto dateTimeToPutInCache)
                     {
                         if (auto variable = varW.lock()) {
                             auto varIdentifier = impl->m_VariableToIdentifierMap.at(variable);
                             if (varIdentifier == identifier) {
                                 impl->m_VariableCacheController->addDateTime(variable, dateTimeToPutInCache);
                                 variable->setDataSeries(dataSeriesAcquired);
                             }
                         }
                     };
                     connect(provider.get(), &IDataProvider::dataProvided, addDateTimeAcquired);
                     this->onRequestDataLoading(newVariable, dateTime);
                 }
             }
             void VariableController::onDateTimeOnSelection(const SqpDateTime &dateTime)
             {
                 qCDebug(LOG_VariableController()) << "VariableController::onDateTimeOnSelection"
                                                   << QThread::currentThread()->objectName();
                 auto selectedRows = impl->m_VariableSelectionModel->selectedRows();
                 for (const auto &selectedRow : qAsConst(selectedRows)) {
                     if (auto selectedVariable = impl->m_VariableModel->variable(selectedRow.row())) {
                         selectedVariable->setDateTime(dateTime);
                         this->onRequestDataLoading(selectedVariable, dateTime);
                     }
                 }
             }
             void VariableController::onVariableRetrieveDataInProgress(QUuid identifier, double progress)
             {
                 auto findReply = [identifier](const auto &entry) { return identifier == entry.second; };
                 auto end = impl->m_VariableToIdentifierMap.cend();
                 auto it = std::find_if(impl->m_VariableToIdentifierMap.cbegin(), end, findReply);
                 if (it != end) {
                     impl->m_VariableModel->setDataProgress(it->first, progress);
                 }
             }
             void VariableController::onAbortProgressRequested(std::shared_ptr<Variable> variable)
             {
                 qCDebug(LOG_VariableController()) << "TORM: VariableController::onAbortProgressRequested"
                                                   << QThread::currentThread()->objectName();
-                auto it = impl->m_VariableToIdentifier.find(variable);
+                auto it = impl->m_VariableToIdentifierMap.find(variable);
-                if (it != impl->m_VariableToIdentifier.cend()) {
+                if (it != impl->m_VariableToIdentifierMap.cend()) {
                     impl->m_VariableToProviderMap.at(variable)->requestDataAborting(it->second);
                 }
                 else {
                     qCWarning(LOG_VariableController())
                         << tr("Aborting progression of inexistant variable detected !!!")
                         << QThread::currentThread()->objectName();
                 }
             }
             void VariableController::onRequestDataLoading(std::shared_ptr<Variable> variable,
                                                           const SqpDateTime &dateTime)
             {
                 qCDebug(LOG_VariableController()) << "VariableController::onRequestDataLoading"
                                                   << QThread::currentThread()->objectName();
                 // we want to load data of the variable for the dateTime.
                 // First we check if the cache contains some of them.
                 // For the other, we ask the provider to give them.
                 if (variable) {
                     auto dateTimeListNotInCache
                         = impl->m_VariableCacheController->provideNotInCacheDateTimeList(variable, dateTime);
                     if (!dateTimeListNotInCache.empty()) {
                         // Ask the provider for each data on the dateTimeListNotInCache
                         auto identifier = impl->m_VariableToIdentifierMap.at(variable);
                         impl->m_VariableToProviderMap.at(variable)->requestDataLoading(
                             identifier,
                             DataProviderParameters{std::move(dateTimeListNotInCache), variable->metadata()});
                     }
                     else {
                         emit variable->updated();
                     }
                 }
                 else {
                     qCCritical(LOG_VariableController()) << tr("Impossible to load data of a variable null");
                 }
             }
             void VariableController::initialize()
             {
                 qCDebug(LOG_VariableController()) << tr("VariableController init") << QThread::currentThread();
                 impl->m_WorkingMutex.lock();
                 qCDebug(LOG_VariableController()) << tr("VariableController init END");
             }
             void VariableController::finalize()
             {
                 impl->m_WorkingMutex.unlock();
             }
             void VariableController::waitForFinish()
             {
                 QMutexLocker locker{&impl->m_WorkingMutex};
             }

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages