ContentManagerModel accesses ContentManager directly

Indirect access from ContentManagerModel to ContentManager (via
KiwixApp) contained a race condition that could lead to a crash.

Here is the call chain:

- In KiwixApp::init() the KiwixApp::mp_manager data member is initialized
  as follows:

    mp_manager = new ContentManager(&m_library, mp_downloader);

- ContentManager's constructor creates the model and the view:

    mp_view = new ContentManagerView();
    managerModel = new ContentManagerModel(this);
    updateModel();
    auto treeView = mp_view->getView();
    treeView->setModel(managerModel);
    treeView->show();

- This starts a cascade of asynchronous events that will eventually
  result in ContentManagerModel::sort() being executed. The latter has
  to call ContentManager::sortBy() and if it does so indirectly via
  `KiwixApp::getContentManager()` before the constructor of
  ContentManager has completed (and KiwixApp::mp_manager has been
  assigned to) a crash is imminent.

This commit eliminates that issue.
This commit is contained in:
Veloman Yunkan 2024-05-17 12:04:58 +04:00
parent 83d72d3b5a
commit b91712422b
2 changed files with 7 additions and 4 deletions

View File

@ -7,8 +7,9 @@
#include "kiwixapp.h" #include "kiwixapp.h"
#include <kiwix/tools.h> #include <kiwix/tools.h>
ContentManagerModel::ContentManagerModel(QObject *parent) ContentManagerModel::ContentManagerModel(ContentManager *contentMgr)
: QAbstractItemModel(parent) : QAbstractItemModel(contentMgr)
, m_contentMgr(*contentMgr)
{ {
connect(&td, &ThumbnailDownloader::oneThumbnailDownloaded, this, &ContentManagerModel::updateImage); connect(&td, &ThumbnailDownloader::oneThumbnailDownloaded, this, &ContentManagerModel::updateImage);
} }
@ -225,7 +226,7 @@ void ContentManagerModel::sort(int column, Qt::SortOrder order)
default: default:
sortBy = "unsorted"; sortBy = "unsorted";
} }
KiwixApp::instance()->getContentManager()->setSortBy(sortBy, order == Qt::AscendingOrder); m_contentMgr.setSortBy(sortBy, order == Qt::AscendingOrder);
} }
RowNode* ContentManagerModel::getRowNode(size_t row) RowNode* ContentManagerModel::getRowNode(size_t row)

View File

@ -11,6 +11,7 @@
#include "rownode.h" #include "rownode.h"
#include <memory> #include <memory>
class ContentManager;
class RowNode; class RowNode;
class Node; class Node;
class DescriptionNode; class DescriptionNode;
@ -58,7 +59,7 @@ public: // types
public: // functions public: // functions
explicit ContentManagerModel(QObject *parent = nullptr); explicit ContentManagerModel(ContentManager* contentMgr);
~ContentManagerModel(); ~ContentManagerModel();
QVariant data(const QModelIndex &index, int role) const override; QVariant data(const QModelIndex &index, int role) const override;
@ -89,6 +90,7 @@ private: // functions
RowNode* getRowNode(size_t row); RowNode* getRowNode(size_t row);
private: // data private: // data
ContentManager& m_contentMgr;
std::shared_ptr<RowNode> rootNode; std::shared_ptr<RowNode> rootNode;
mutable ThumbnailDownloader td; mutable ThumbnailDownloader td;
QMap<QString, size_t> bookIdToRowMap; QMap<QString, size_t> bookIdToRowMap;