Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Thread safe plugin system #1068

Merged
merged 3 commits into from Oct 14, 2013
Merged

Thread safe plugin system #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b500156
Changed PluginFactoryBase::*findPMaker to return void*
Dr15Jones Oct 10, 2013
a7fb8cd
Member data m_plugins was moved from protected to private
Dr15Jones Oct 11, 2013
2b96b07
Use mutex to protect Plugin system
Dr15Jones Oct 11, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions FWCore/PluginManager/interface/PluginFactory.h
View file
Open in desktop
Expand Up @@ -56,16 +56,16 @@ class PluginFactory<R*(Args...)> : public PluginFactoryBase
virtual const std::string& category() const ;

R* create(const std::string& iName, Args... args) const {
return reinterpret_cast<PMakerBase*>(PluginFactoryBase::findPMaker(iName)->second.front().first)->create(std::forward<Args>(args)...);
return reinterpret_cast<PMakerBase*>(PluginFactoryBase::findPMaker(iName))->create(std::forward<Args>(args)...);
}

///like above but returns 0 if iName is unknown
R* tryToCreate(const std::string& iName, Args... args) const {
typename Plugins::const_iterator itFound = PluginFactoryBase::tryToFindPMaker(iName);
if(itFound ==m_plugins.end() ) {
return 0;
auto found = PluginFactoryBase::tryToFindPMaker(iName);
if(found ==nullptr) {
return nullptr;
}
return reinterpret_cast<PMakerBase*>(itFound->second.front().first)->create(args...);
return reinterpret_cast<PMakerBase*>(found)->create(args...);
}
// ---------- static member functions --------------------

Expand Down
11 changes: 7 additions & 4 deletions FWCore/PluginManager/interface/PluginFactoryBase.h
View file
Open in desktop
Expand Up @@ -23,6 +23,8 @@
#include <string>
#include <vector>
#include <map>
#include <mutex>

#include "FWCore/Utilities/interface/Signal.h"
// user include files
#include "FWCore/PluginManager/interface/PluginInfo.h"
Expand Down Expand Up @@ -66,10 +68,10 @@ class PluginFactoryBase
//since each inheriting class has its own Container type to hold their PMakers
// this function allows them to share the same code when doing the lookup
// this routine will throw an exception if iName is unknown therefore the return value is always valid
Plugins::const_iterator findPMaker(const std::string& iName) const;
void* findPMaker(const std::string& iName) const;

//similar to findPMaker but will return 'end()' if iName is known
Plugins::const_iterator tryToFindPMaker(const std::string& iName) const;
void* tryToFindPMaker(const std::string& iName) const;

void fillInfo(const PMakers &makers,
PluginInfo& iInfo,
Expand All @@ -79,15 +81,16 @@ class PluginFactoryBase

void registerPMaker(void* iPMaker, const std::string& iName);

Plugins m_plugins;

private:
PluginFactoryBase(const PluginFactoryBase&); // stop default

const PluginFactoryBase& operator=(const PluginFactoryBase&); // stop default

void checkProperLoadable(const std::string& iName, const std::string& iLoadedFrom) const;
// ---------- member data --------------------------------
Plugins m_plugins;
mutable std::recursive_mutex m_mutex;


};

Expand Down
12 changes: 8 additions & 4 deletions FWCore/PluginManager/src/PluginFactoryBase.cc
View file
Open in desktop
Expand Up @@ -72,9 +72,10 @@ PluginFactoryBase::newPlugin(const std::string& iName)
}


PluginFactoryBase::Plugins::const_iterator
void*
PluginFactoryBase::findPMaker(const std::string& iName) const
{
std::lock_guard<std::recursive_mutex> guard(m_mutex);
//do we already have it?
Plugins::const_iterator itFound = m_plugins.find(iName);
if(itFound == m_plugins.end()) {
Expand All @@ -87,13 +88,14 @@ PluginFactoryBase::findPMaker(const std::string& iName) const
} else {
checkProperLoadable(iName,itFound->second.front().second);
}
return itFound;
return itFound->second.front().first;
}


PluginFactoryBase::Plugins::const_iterator
void*
PluginFactoryBase::tryToFindPMaker(const std::string& iName) const
{
std::lock_guard<std::recursive_mutex> guard(m_mutex);
//do we already have it?
Plugins::const_iterator itFound = m_plugins.find(iName);
if(itFound == m_plugins.end()) {
Expand All @@ -109,7 +111,7 @@ PluginFactoryBase::tryToFindPMaker(const std::string& iName) const
} else {
checkProperLoadable(iName,itFound->second.front().second);
}
return itFound;
return itFound != m_plugins.end()? itFound->second.front().first : nullptr;
}

void
Expand All @@ -127,6 +129,7 @@ PluginFactoryBase::fillInfo(const PMakers &makers,
void
PluginFactoryBase::fillAvailable(std::vector<PluginInfo>& iReturn) const {
PluginInfo info;
std::lock_guard<std::recursive_mutex> guard(m_mutex);
for( Plugins::const_iterator it = m_plugins.begin();
it != m_plugins.end();
++it) {
Expand Down Expand Up @@ -156,6 +159,7 @@ PluginFactoryBase::checkProperLoadable(const std::string& iName, const std::stri

void
PluginFactoryBase::registerPMaker(void* iPMaker, const std::string& iName) {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
m_plugins[iName].push_back(std::pair<void*,std::string>(iPMaker,PluginManager::loadingFile()));
newPlugin(iName);
}
Expand Down
14 changes: 13 additions & 1 deletion FWCore/PluginManager/src/PluginManager.cc
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@

#include <fstream>
#include <set>
#include <mutex>

// user include files
#include "FWCore/PluginManager/interface/PluginManager.h"
Expand Down Expand Up @@ -136,6 +137,8 @@ namespace {
return iLHS.name_ < iRHS.name_;
}
};

std::mutex s_pluginLoadMutex;
}

const boost::filesystem::path&
Expand Down Expand Up @@ -229,6 +232,9 @@ PluginManager::load(const std::string& iCategory,
askedToLoadCategoryWithPlugin_(iCategory,iPlugin);
const boost::filesystem::path& p = loadableFor(iCategory,iPlugin);

//Need to make sure we only have on SharedLibrary loading at a time
// plus loadables_ must be serialized
std::lock_guard<std::mutex> guard(s_pluginLoadMutex);
//have we already loaded this?
std::map<boost::filesystem::path, boost::shared_ptr<SharedLibrary> >::iterator itLoaded =
loadables_.find(p);
Expand Down Expand Up @@ -257,6 +263,10 @@ PluginManager::tryToLoad(const std::string& iCategory,
return 0;
}

//Need to make sure we only have on SharedLibrary loading at a time
// plus loadables_ must be serialized
std::lock_guard<std::mutex> guard(s_pluginLoadMutex);

//have we already loaded this?
std::map<boost::filesystem::path, boost::shared_ptr<SharedLibrary> >::iterator itLoaded =
loadables_.find(p);
Expand Down Expand Up @@ -306,13 +316,15 @@ PluginManager::configure(const Config& iConfig )
const std::string&
PluginManager::staticallyLinkedLoadingFileName()
{
static std::string s_name("static");
static const std::string s_name("static");
return s_name;
}

std::string&
PluginManager::loadingLibraryNamed_()
{
//NOTE: s_pluginLoadMutex indirectly guards this since this value
// is only accessible via the Sentry call which us guarded by the mutex
static std::string s_name(staticallyLinkedLoadingFileName());
return s_name;
}
Expand Down