From b059bcdc52e14150dd1673157ba738715f826e32 Mon Sep 17 00:00:00 2001 From: Loic Dachary Date: Sat, 23 Aug 2014 11:07:29 +0200 Subject: [PATCH] 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 --- src/erasure-code/ErasureCodePlugin.cc | 5 +++++ src/test/erasure-code/TestErasureCodePlugin.cc | 11 +++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/erasure-code/ErasureCodePlugin.cc b/src/erasure-code/ErasureCodePlugin.cc index 297b3c82cef05..31abaf7ad75aa 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::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 plugins_list; get_str_list(plugins, plugins_list); for (list::iterator i = plugins_list.begin(); diff --git a/src/test/erasure-code/TestErasureCodePlugin.cc b/src/test/erasure-code/TestErasureCodePlugin.cc index e3d814cb1fbc1..ebd0d6481f2d5 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) { -- 2.39.5