Commit e0039d26 authored by Aleksandr Konstantinov's avatar Aleksandr Konstantinov
Browse files

Attempt to avoid reace condition while reloading module after probing. Needs additional testing.

parent 763e1268
...@@ -47,15 +47,23 @@ ModuleManager::~ModuleManager(void) ...@@ -47,15 +47,23 @@ ModuleManager::~ModuleManager(void)
for(plugin_cache_t::iterator i = plugin_cache.begin(); i != plugin_cache.end();) { for(plugin_cache_t::iterator i = plugin_cache.begin(); i != plugin_cache.end();) {
while(i->second.unload(this) > 0) { }; while(i->second.unload(this) > 0) { };
if(i->second) { if(i->second) {
// module is on unloaded only if it is in use according to usage counter // module is not unloaded only if it is in use according to usage counter
++i; ++i;
} else { } else {
plugin_cache.erase(i); plugin_cache.erase(i);
i = plugin_cache.begin(); // for map erase does not return iterator i = plugin_cache.begin(); // for map erase does not return iterator
} }
} }
for(plugin_trash_t::iterator i = plugin_trash.begin(); i != plugin_trash.end();) {
while(i->unload(this) > 0) { };
if(*i) {
++i;
} else {
i = plugin_trash.erase(i);
}
}
// exit only when all plugins unloaded // exit only when all plugins unloaded
if(plugin_cache.empty()) return; if(plugin_cache.empty() && plugin_trash.empty()) return;
// otherwise wait for plugins to be released // otherwise wait for plugins to be released
logger.msg(WARNING, "Busy plugins found while unloading Module Manager. Waiting for them to be released."); logger.msg(WARNING, "Busy plugins found while unloading Module Manager. Waiting for them to be released.");
for(;;) { for(;;) {
...@@ -74,7 +82,15 @@ ModuleManager::~ModuleManager(void) ...@@ -74,7 +82,15 @@ ModuleManager::~ModuleManager(void)
i = plugin_cache.begin(); // for map erase does not return iterator i = plugin_cache.begin(); // for map erase does not return iterator
} }
} }
if(plugin_cache.empty()) return; for(plugin_trash_t::iterator i = plugin_trash.begin(); i != plugin_trash.end();) {
while(i->unload(this) > 0) { };
if(*i) {
++i;
} else {
i = plugin_trash.erase(i);
}
}
if(plugin_cache.empty() && plugin_trash.empty()) return;
}; };
} }
...@@ -105,22 +121,25 @@ void ModuleManager::unload(Glib::Module *module) ...@@ -105,22 +121,25 @@ void ModuleManager::unload(Glib::Module *module)
p!=plugin_cache.end();++p) { p!=plugin_cache.end();++p) {
if(p->second == module) { if(p->second == module) {
p->second.unload(this); p->second.unload(this);
if(!(p->second)) plugin_cache.erase(p); if(!(p->second)) {
break; plugin_cache.erase(p);
}
return;
} }
} }
} for(plugin_trash_t::iterator p = plugin_trash.begin();
p!=plugin_trash.end();++p) {
void ModuleManager::unload(const std::string& name) if(*p == module) {
{ p->unload(NULL); // Do not call destructor for trashed module
Glib::Mutex::Lock lock(mlock); if(!(*p)) {
plugin_cache_t::iterator p = plugin_cache.find(name); plugin_trash.erase(p);
if (p != plugin_cache.end()) { }
p->second.unload(this); return;
if(!(p->second)) plugin_cache.erase(p); }
} }
} }
void ModuleManager::use(Glib::Module *module) void ModuleManager::use(Glib::Module *module)
{ {
Glib::Mutex::Lock lock(mlock); Glib::Mutex::Lock lock(mlock);
...@@ -128,7 +147,14 @@ void ModuleManager::use(Glib::Module *module) ...@@ -128,7 +147,14 @@ void ModuleManager::use(Glib::Module *module)
p!=plugin_cache.end();++p) { p!=plugin_cache.end();++p) {
if(p->second == module) { if(p->second == module) {
p->second.use(); p->second.use();
break; return;
}
}
for(plugin_trash_t::iterator p = plugin_trash.begin();
p!=plugin_trash.end();++p) {
if(*p == module) {
p->use();
return;
} }
} }
} }
...@@ -140,8 +166,20 @@ void ModuleManager::unuse(Glib::Module *module) ...@@ -140,8 +166,20 @@ void ModuleManager::unuse(Glib::Module *module)
p!=plugin_cache.end();++p) { p!=plugin_cache.end();++p) {
if(p->second == module) { if(p->second == module) {
p->second.unuse(); p->second.unuse();
if(!(p->second)) plugin_cache.erase(p); if(!(p->second)) {
break; plugin_cache.erase(p);
}
return;
}
}
for(plugin_trash_t::iterator p = plugin_trash.begin();
p!=plugin_trash.end();++p) {
if(*p == module) {
p->unuse();
if(!(*p)) {
plugin_trash.erase(p);
}
return;
} }
} }
} }
...@@ -208,6 +246,7 @@ Glib::Module* ModuleManager::reload(Glib::Module* omodule) ...@@ -208,6 +246,7 @@ Glib::Module* ModuleManager::reload(Glib::Module* omodule)
if(p->second == omodule) break; if(p->second == omodule) break;
} }
if(p==plugin_cache.end()) return NULL; if(p==plugin_cache.end()) return NULL;
// TODO: avoid reloading modules which are already properly loaded
Glib::ModuleFlags flags = Glib::ModuleFlags(0); Glib::ModuleFlags flags = Glib::ModuleFlags(0);
//flags|=Glib::MODULE_BIND_LOCAL; //flags|=Glib::MODULE_BIND_LOCAL;
Glib::Module *module = new Glib::Module(omodule->get_name(),flags); Glib::Module *module = new Glib::Module(omodule->get_name(),flags);
...@@ -216,8 +255,14 @@ Glib::Module* ModuleManager::reload(Glib::Module* omodule) ...@@ -216,8 +255,14 @@ Glib::Module* ModuleManager::reload(Glib::Module* omodule)
if(module) delete module; if(module) delete module;
return NULL; return NULL;
} }
p->second=module; // Move existing module into trash list for later removal and
delete omodule; // store handle in current entry.
// Trashed module keeps load counter. But usage counter stays in cached entry.
LoadableModuleDescription trashed;
p->second.shift(module, trashed);
if (trashed) {
plugin_trash.push_back(trashed);
}
return module; return module;
} }
...@@ -327,5 +372,14 @@ int ModuleManager::LoadableModuleDescription::unload(ModuleManager* manager) { ...@@ -327,5 +372,14 @@ int ModuleManager::LoadableModuleDescription::unload(ModuleManager* manager) {
return count; return count;
} }
void ModuleManager::LoadableModuleDescription::shift(Glib::Module* source, LoadableModuleDescription& target) {
target.module = module;
target.count = count;
module = source;
count = 0;
load(); // accepting new module handler
target.unload(NULL); // removing reference taken by new module
}
} // namespace Arc } // namespace Arc
...@@ -72,7 +72,6 @@ class ModuleManager ...@@ -72,7 +72,6 @@ class ModuleManager
Glib::Module* module; Glib::Module* module;
int count; int count;
int usage_count; int usage_count;
std::string path;
void check_unload(ModuleManager* manager); void check_unload(ModuleManager* manager);
public: public:
LoadableModuleDescription(void); LoadableModuleDescription(void);
...@@ -83,6 +82,7 @@ class ModuleManager ...@@ -83,6 +82,7 @@ class ModuleManager
operator bool(void) { return (module != NULL); }; operator bool(void) { return (module != NULL); };
bool operator!(void) { return (module == NULL); }; bool operator!(void) { return (module == NULL); };
bool operator==(Glib::Module* m) { return (module==m); }; bool operator==(Glib::Module* m) { return (module==m); };
void shift(Glib::Module* source, LoadableModuleDescription& target);
int load(void); int load(void);
int unload(ModuleManager* manager); int unload(ModuleManager* manager);
int use(void) { ++usage_count; return usage_count; }; int use(void) { ++usage_count; return usage_count; };
...@@ -97,19 +97,19 @@ class ModuleManager ...@@ -97,19 +97,19 @@ class ModuleManager
friend class LoadableModuleDescription; friend class LoadableModuleDescription;
typedef std::map<std::string, LoadableModuleDescription> plugin_cache_t; typedef std::map<std::string, LoadableModuleDescription> plugin_cache_t;
typedef std::list<LoadableModuleDescription> plugin_trash_t;
Glib::Mutex mlock; Glib::Mutex mlock;
static Logger logger; static Logger logger;
std::list<std::string> plugin_dir; /** collection of path to directory for modules */ std::list<std::string> plugin_dir; /** collection of path to directory for modules */
plugin_cache_t plugin_cache; /** Cache of handles of loaded modules */ plugin_cache_t plugin_cache; /** Cache of handles of loaded modules */
plugin_trash_t plugin_trash; /** Trash bin of reloaded modules */
ModuleManager(const ModuleManager&) {}; ModuleManager(const ModuleManager&) {};
ModuleManager& operator=(const ModuleManager&) { return *this; }; ModuleManager& operator=(const ModuleManager&) { return *this; };
protected: protected:
/** Unload module by its identifier. /** Unload module by its identifier.
Decreases load counter and unloads module when it reaches 0. */ Decreases load counter and unloads module when it reaches 0. */
void unload(Glib::Module* module); void unload(Glib::Module* module);
/** Unload module by its name */
void unload(const std::string& name);
public: public:
/** Constructor. /** Constructor.
It is supposed to process correponding configuration subtree It is supposed to process correponding configuration subtree
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment