Skip to content

Commit

Permalink
Fix race conditions in the PluginSystem
Browse files Browse the repository at this point in the history
- Use tbb::zero_allocator to be sure std::atomic is nullptr even before its constructor has finished
- Avoid having a temporary empty std::shared_ptr be placed in the map before being reset
- Be sure object is fully initialized before using it
  • Loading branch information
Dr15Jones committed Dec 1, 2020
1 parent 398e2ae commit 82c4e10
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 2 deletions.
2 changes: 1 addition & 1 deletion FWCore/PluginManager/interface/PluginFactoryBase.h
Expand Up @@ -61,7 +61,7 @@ namespace edmplugin {
std::atomic<void*> m_ptr;
};

typedef tbb::concurrent_vector<PluginMakerInfo> PMakers;
typedef tbb::concurrent_vector<PluginMakerInfo, tbb::zero_allocator<PluginMakerInfo>> PMakers;
typedef tbb::concurrent_unordered_map<std::string, PMakers> Plugins;

// ---------- const member functions ---------------------
Expand Down
4 changes: 4 additions & 0 deletions FWCore/PluginManager/src/PluginFactoryBase.cc
Expand Up @@ -75,6 +75,10 @@ namespace edmplugin {
<< "'\n but was not there. This means the plugin cache is incorrect. Please run 'EdmPluginRefresh " << lib
<< "'";
}
//The item in the container can still be under construction so wait until the m_ptr has been set since that is done last
auto const& value = itFound->second.front();
while (value.m_ptr.load(std::memory_order_acquire) == nullptr) {
}
} else {
//The item in the container can still be under construction so wait until the m_ptr has been set since that is done last
auto const& value = itFound->second.front();
Expand Down
2 changes: 1 addition & 1 deletion FWCore/PluginManager/src/PluginManager.cc
Expand Up @@ -251,7 +251,7 @@ namespace edmplugin {
throw;
}
}
loadables_[p] = ptr;
loadables_.emplace(p, ptr);
justLoaded_(*ptr);
return *ptr;
}
Expand Down

0 comments on commit 82c4e10

Please sign in to comment.