Skip to content

Commit

Permalink
ConfigItem: Fix infinite recursion caused by ignore_on_error when c…
Browse files Browse the repository at this point in the history
…ommitting an item

When committing an item with `ignore_on_error` flag set fails, the `Commit()` method only returns `nullptr`
and the current item is not being dropped from `m_Items`. `CommittNewItems()` also doesn't check the return
value of `Commit()` but just continues and tries to commit all items from `m_Items` in recursive call. Since
this corrupt item is never removed from `m_Items`, it ends up in an endless recursion till it finally crashes.
  • Loading branch information
yhabteab committed Jun 20, 2022
1 parent 4522522 commit 4f7edd6
Showing 1 changed file with 15 additions and 6 deletions.
21 changes: 15 additions & 6 deletions lib/config/configitem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,9 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
<< "Committing " << items.size() << " new items.";
#endif /* I2_DEBUG */

for (const auto& ip : items)
newItems.push_back(ip.first);

std::set<Type::Ptr> types;
std::set<Type::Ptr> completed_types;
std::atomic<int> itemsCount(0);

for (const Type::Ptr& type : Type::GetAllTypes()) {
if (ConfigObject::TypeInstance->IsAssignableFrom(type))
Expand All @@ -464,14 +462,25 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
continue;

std::atomic<int> committed_items(0);
upq.ParallelFor(items, [&type, &committed_items](const ItemPair& ip) {
upq.ParallelFor(items, [&type, &committed_items, &itemsCount, &newItems](const ItemPair& ip) {
const ConfigItem::Ptr& item = ip.first;

if (item->m_Type != type)
return;

ip.first->Commit(ip.second);
if (item->Commit(ip.second) == nullptr) {
if (item->IsIgnoreOnError()) {
item->Unregister();
}

return;
}

itemsCount++;
committed_items++;

std::unique_lock<std::mutex> lock(m_Mutex);
newItems.emplace_back(item);
});

upq.Join();
Expand All @@ -491,7 +500,7 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue

#ifdef I2_DEBUG
Log(LogDebug, "configitem")
<< "Committed " << items.size() << " items.";
<< "Committed " << itemsCount.load() << " items.";
#endif /* I2_DEBUG */

completed_types.clear();
Expand Down

0 comments on commit 4f7edd6

Please sign in to comment.