From: Loic Dachary <loic-201408@dachary.org>
Date: Sat, 23 Aug 2014 09:07:29 +0000 (+0200)
Subject: erasure-code: assert the PluginRegistry lock is held when it must
X-Git-Tag: v0.86~193^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F2299%2Fhead;p=ceph.git

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 297b3c82cef..31abaf7ad75 100644
--- 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 e3d814cb1fb..ebd0d6481f2 100644
--- 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) {