diff --git a/src/libs/solutions/tasking/tasktree.cpp b/src/libs/solutions/tasking/tasktree.cpp index 37bc8385e21..84e6f544553 100644 --- a/src/libs/solutions/tasking/tasktree.cpp +++ b/src/libs/solutions/tasking/tasktree.cpp @@ -865,6 +865,11 @@ void *TreeStorageBase::activeStorageVoid() const return it.value(); } +int TreeStorageBase::activeStorageId() const +{ + return m_storageData->m_activeStorage; +} + int TreeStorageBase::createStorage() const { QTC_ASSERT(m_storageData->m_constructor, return 0); // TODO: add isValid()? @@ -895,6 +900,7 @@ void TreeStorageBase::activateStorage(int id) const return; } QTC_ASSERT(m_storageData->m_activeStorage == 0, return); + // TODO: Unneeded check? OTOH, it's quite important... const auto it = m_storageData->m_storageHash.find(id); QTC_ASSERT(it != m_storageData->m_storageHash.end(), return); m_storageData->m_activeStorage = id; @@ -946,7 +952,15 @@ void GroupItem::addChildren(const QList &children) m_children.append(child); break; case Type::Storage: - m_storageList.append(child.m_storageList); + // Check for duplicates, as can't have the same storage twice on the same level. + for (const TreeStorageBase &storage : child.m_storageList) { + if (m_storageList.contains(storage)) { + QTC_ASSERT(false, qWarning("Can't add the same storage into one Group twice, " + "skipping...")); + continue; + } + m_storageList.append(storage); + } break; } } @@ -985,7 +999,6 @@ public: void emitStartedAndProgress(); void emitProgress(); void emitDone(DoneWith result); - QList addStorages(const QList &storages); void callSetupHandler(TreeStorageBase storage, int storageId) { callStorageHandler(storage, storageId, &StorageHandler::m_setupHandler); } @@ -1151,45 +1164,33 @@ void TaskTreePrivate::emitDone(DoneWith result) emit q->done(result); } -QList TaskTreePrivate::addStorages(const QList &storages) -{ - QList addedStorages; - for (const TreeStorageBase &storage : storages) { - QTC_ASSERT(!m_storages.contains(storage), qWarning("Can't add the same storage into " - "one TaskTree twice, skipping..."); continue); - addedStorages << storage; - m_storages << storage; - } - return addedStorages; -} - class ExecutionContextActivator { public: - ExecutionContextActivator(TaskContainer *container) - : m_container(container) { activateContext(m_container); } - ~ExecutionContextActivator() { deactivateContext(m_container); } + ExecutionContextActivator(TaskContainer *container) { activateContext(container); } + ~ExecutionContextActivator() { + for (int i = m_activeStorages.size() - 1; i >= 0; --i) // iterate in reverse order + m_activeStorages[i].activateStorage(0); + } private: - static void activateContext(TaskContainer *container) + void activateContext(TaskContainer *container) { QTC_ASSERT(container && container->isRunning(), return); const TaskContainer::ConstData &constData = container->m_constData; + for (int i = 0; i < constData.m_storageList.size(); ++i) { + const TreeStorageBase &storage = constData.m_storageList[i]; + if (storage.activeStorageId()) + continue; // Storage shadowing: The storage is already active, skipping it... + m_activeStorages.append(storage); + storage.activateStorage(container->m_runtimeData->m_storageIdList.value(i)); + } + // Go to the parent after activating this storages so that storage shadowing works + // in the direction from child to parent root. if (constData.m_parentContainer) activateContext(constData.m_parentContainer); - for (int i = 0; i < constData.m_storageList.size(); ++i) - constData.m_storageList[i].activateStorage(container->m_runtimeData->m_storageIdList.value(i)); } - static void deactivateContext(TaskContainer *container) - { - QTC_ASSERT(container && container->isRunning(), return); - const TaskContainer::ConstData &constData = container->m_constData; - for (int i = constData.m_storageList.size() - 1; i >= 0; --i) // iterate in reverse order - constData.m_storageList[i].activateStorage(0); - if (constData.m_parentContainer) - deactivateContext(constData.m_parentContainer); - } - TaskContainer *m_container = nullptr; + QList m_activeStorages; }; template addStorages(task.m_storageList)) + , m_storageList(task.m_storageList) , m_children(createChildren(taskTreePrivate, thisContainer, task.m_children)) , m_taskCount(std::accumulate(m_children.cbegin(), m_children.cend(), 0, [](int r, TaskNode *n) { return r + n->taskCount(); })) -{} +{ + for (const TreeStorageBase &storage : m_storageList) + m_taskTreePrivate->m_storages << storage; +} QList TaskContainer::RuntimeData::createStorages(const TaskContainer::ConstData &constData) { @@ -2170,6 +2174,7 @@ void TaskTree::setRecipe(const Group &recipe) QTC_ASSERT(!isRunning(), qWarning("The TaskTree is already running, ignoring..."); return); QTC_ASSERT(!d->m_guard.isLocked(), qWarning("The setRecipe() is called from one of the" "TaskTree handlers, ignoring..."); return); + // TODO: Should we clear the m_storageHandlers, too? d->m_storages.clear(); d->m_root.reset(new TaskNode(d, recipe, nullptr)); } diff --git a/src/libs/solutions/tasking/tasktree.h b/src/libs/solutions/tasking/tasktree.h index d6375023b9f..a575d7c10c9 100644 --- a/src/libs/solutions/tasking/tasktree.h +++ b/src/libs/solutions/tasking/tasktree.h @@ -52,6 +52,7 @@ private: TreeStorageBase(StorageConstructor ctor, StorageDestructor dtor); void *activeStorageVoid() const; + int activeStorageId() const; int createStorage() const; void deleteStorage(int id) const; diff --git a/tests/auto/solutions/tasking/tst_tasking.cpp b/tests/auto/solutions/tasking/tst_tasking.cpp index ae0b3d63329..994d3d54967 100644 --- a/tests/auto/solutions/tasking/tst_tasking.cpp +++ b/tests/auto/solutions/tasking/tst_tasking.cpp @@ -4,6 +4,7 @@ #include #include +#include using namespace Tasking; @@ -39,7 +40,8 @@ enum class Handler { TweakDoneToError, Sync, BarrierAdvance, - Timeout + Timeout, + Storage }; Q_ENUM_NS(Handler); @@ -2425,6 +2427,81 @@ void tst_Tasking::testTree_data() QTest::newRow("GroupDoneWithTimeoutHandler") << TestData{storage, root4, doneLog, 2, OnDone::Success}; } + + { + // This test check if storage shadowing works OK. + + // This helper storage collect the pointers to storages created by shadowedStorage. + const TreeStorage> helperStorage; // One instance in this test. + // This storage is repeated in nested groups, the innermost storage will shadow outer ones. + const TreeStorage shadowedStorage; // Three instances in this test. + + const auto groupSetupWithStorage = [storage, helperStorage, shadowedStorage](int taskId) { + return onGroupSetup([storage, helperStorage, shadowedStorage, taskId] { + storage->m_log.append({taskId, Handler::GroupSetup}); + helperStorage->insert(taskId, shadowedStorage.activeStorage()); + *shadowedStorage = taskId; + }); + }; + const auto groupDoneWithStorage = [storage, helperStorage, shadowedStorage](int taskId) { + return onGroupDone([storage, helperStorage, shadowedStorage, taskId](DoneWith result) { + storage->m_log.append({taskId, resultToGroupHandler(result)}); + auto it = helperStorage->find(taskId); + if (it == helperStorage->end()) { + qWarning() << "The helperStorage is missing the shadowedStorage."; + return; + } else if (*it != shadowedStorage.activeStorage()) { + qWarning() << "Wrong active instance of the shadowedStorage."; + return; + } else if (**it != taskId) { + qWarning() << "Wrong data of the active instance of the shadowedStorage."; + return; + } + helperStorage->erase(it); + storage->m_log.append({*shadowedStorage, Handler::Storage}); + }); + }; + + const Group root { + Storage(storage), + Storage(helperStorage), + Storage(shadowedStorage), + groupSetupWithStorage(1), + Group { + Storage(shadowedStorage), + groupSetupWithStorage(2), + Group { + Storage(shadowedStorage), + groupSetupWithStorage(3), + groupDoneWithStorage(3) + }, + Group { + Storage(shadowedStorage), + groupSetupWithStorage(4), + groupDoneWithStorage(4) + }, + groupDoneWithStorage(2) + }, + groupDoneWithStorage(1) + }; + + const Log log { + {1, Handler::GroupSetup}, + {2, Handler::GroupSetup}, + {3, Handler::GroupSetup}, + {3, Handler::GroupSuccess}, + {3, Handler::Storage}, + {4, Handler::GroupSetup}, + {4, Handler::GroupSuccess}, + {4, Handler::Storage}, + {2, Handler::GroupSuccess}, + {2, Handler::Storage}, + {1, Handler::GroupSuccess}, + {1, Handler::Storage}, + }; + + QTest::newRow("StorageShadowing") << TestData{storage, root, log, 0, OnDone::Success}; + } } void tst_Tasking::testTree()