erasure-code: assert the PluginRegistry lock is held when it must

Add lock to the preload method and assert that it is held by methods
requiring it. Although preload is called at bootstrap and does not
require the lock, adding it does not hurt and makes the lock policy
clearer to understand.

Signed-off-by: Loic Dachary <loic-201408@dachary.org>

diff --git a/src/erasure-code/ErasureCodePlugin.cc b/src/erasure-code/ErasureCodePlugin.cc
index 297b3c82cef0520af3d3f7ab52a6190a590929f1..31abaf7ad75aae25c3d9f7b7c3624ac34081518a 100644 (file)
--- a/src/erasure-code/ErasureCodePlugin.cc
+++ b/src/erasure-code/ErasureCodePlugin.cc
@@ -53,6 +53,7 @@ ErasureCodePluginRegistry::~ErasureCodePluginRegistry()
 
 int ErasureCodePluginRegistry::remove(const std::string &name)
 {
+  assert(lock.is_locked());
   if (plugins.find(name) == plugins.end())
     return -ENOENT;
   std::map<std::string,ErasureCodePlugin*>::iterator plugin = plugins.find(name);
@@ -66,6 +67,7 @@ int ErasureCodePluginRegistry::remove(const std::string &name)
 int ErasureCodePluginRegistry::add(const std::string &name,
                                    ErasureCodePlugin* plugin)
 {
+  assert(lock.is_locked());
   if (plugins.find(name) != plugins.end())
     return -EEXIST;
   plugins[name] = plugin;
@@ -74,6 +76,7 @@ int ErasureCodePluginRegistry::add(const std::string &name,
 
 ErasureCodePlugin *ErasureCodePluginRegistry::get(const std::string &name)
 {
+  assert(lock.is_locked());
   if (plugins.find(name) != plugins.end())
     return plugins[name];
   else
@@ -112,6 +115,7 @@ int ErasureCodePluginRegistry::load(const std::string &plugin_name,
                                    ErasureCodePlugin **plugin,
                                    ostream &ss)
 {
+  assert(lock.is_locked());
   std::string fname = directory + "/" PLUGIN_PREFIX
     + plugin_name + PLUGIN_SUFFIX;
   void *library = dlopen(fname.c_str(), RTLD_NOW);
@@ -170,6 +174,7 @@ int ErasureCodePluginRegistry::preload(const std::string &plugins,
                                       const std::string &directory,
                                       ostream &ss)
 {
+  Mutex::Locker l(lock);
   list<string> plugins_list;
   get_str_list(plugins, plugins_list);
   for (list<string>::iterator i = plugins_list.begin();

diff --git a/src/test/erasure-code/TestErasureCodePlugin.cc b/src/test/erasure-code/TestErasureCodePlugin.cc
index e3d814cb1fbc1f58597be744d94d8b154b8b2dbf..ebd0d6481f2d5563342c1fa9226f877921ff3ac7 100644 (file)
--- a/src/test/erasure-code/TestErasureCodePlugin.cc
+++ b/src/test/erasure-code/TestErasureCodePlugin.cc
@@ -96,10 +96,13 @@ TEST_F(ErasureCodePluginRegistryTest, all)
   EXPECT_EQ(0, instance.factory("example", parameters, &erasure_code, ss));
   EXPECT_TRUE(erasure_code);
   ErasureCodePlugin *plugin = 0;
-  EXPECT_EQ(-EEXIST, instance.load("example", directory, &plugin, ss));
-  EXPECT_EQ(-ENOENT, instance.remove("does not exist"));
-  EXPECT_EQ(0, instance.remove("example"));
-  EXPECT_EQ(0, instance.load("example", directory, &plugin, ss));
+  {
+    Mutex::Locker l(instance.lock);
+    EXPECT_EQ(-EEXIST, instance.load("example", directory, &plugin, ss));
+    EXPECT_EQ(-ENOENT, instance.remove("does not exist"));
+    EXPECT_EQ(0, instance.remove("example"));
+    EXPECT_EQ(0, instance.load("example", directory, &plugin, ss));
+  }
 }
 
 int main(int argc, char **argv) {