You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2015/06/14 11:46:39 UTC
[21/21] mesos git commit: Update ModuleManager to use synchronized.
Update ModuleManager to use synchronized.
Review: https://reviews.apache.org/r/35098
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/98039c99
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/98039c99
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/98039c99
Branch: refs/heads/master
Commit: 98039c99b0a2e0a36a7d4c22e672ecd817923bb4
Parents: 8939609
Author: Joris Van Remoortere <jo...@gmail.com>
Authored: Sat Jun 13 07:15:59 2015 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Sun Jun 14 02:43:01 2015 -0700
----------------------------------------------------------------------
src/module/manager.cpp | 129 ++++++++++++++++++++++----------------------
src/module/manager.hpp | 71 ++++++++++++------------
2 files changed, 101 insertions(+), 99 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/98039c99/src/module/manager.cpp
----------------------------------------------------------------------
diff --git a/src/module/manager.cpp b/src/module/manager.cpp
index 2c7f876..909ca56 100644
--- a/src/module/manager.cpp
+++ b/src/module/manager.cpp
@@ -16,6 +16,7 @@
* limitations under the License.
*/
+#include <mutex>
#include <string>
#include <vector>
@@ -31,8 +32,6 @@
#include <stout/stringify.hpp>
#include <stout/version.hpp>
-#include "common/lock.hpp"
-
#include "messages/messages.hpp"
#include "module/manager.hpp"
@@ -46,7 +45,7 @@ using namespace mesos;
using namespace mesos::internal;
using namespace mesos::modules;
-pthread_mutex_t ModuleManager::mutex = PTHREAD_MUTEX_INITIALIZER;
+std::mutex ModuleManager::mutex;
hashmap<const string, string> ModuleManager::kindToVersion;
hashmap<const string, ModuleBase*> ModuleManager::moduleBases;
hashmap<const string, Owned<DynamicLibrary>> ModuleManager::dynamicLibraries;
@@ -104,15 +103,16 @@ void ModuleManager::initialize()
// of ModuleBases.
Try<Nothing> ModuleManager::unload(const string& moduleName)
{
- Lock lock(&mutex);
- if (!moduleBases.contains(moduleName)) {
- return Error(
- "Error unloading module '" + moduleName + "': module not loaded");
- }
+ synchronized (mutex) {
+ if (!moduleBases.contains(moduleName)) {
+ return Error(
+ "Error unloading module '" + moduleName + "': module not loaded");
+ }
- // Do not remove the dynamiclibrary as it could result in unloading
- // the library from the process memory.
- moduleBases.erase(moduleName);
+ // Do not remove the dynamiclibrary as it could result in
+ // unloading the library from the process memory.
+ moduleBases.erase(moduleName);
+ }
return Nothing();
}
@@ -189,64 +189,67 @@ Try<Nothing> ModuleManager::verifyModule(
Try<Nothing> ModuleManager::load(const Modules& modules)
{
- Lock lock(&mutex);
- initialize();
-
- foreach (const Modules::Library& library, modules.libraries()) {
- string libraryName;
- if (library.has_file()) {
- libraryName = library.file();
- } else if (library.has_name()) {
- libraryName = os::libraries::expandName(library.name());
- } else {
- return Error("Library name or path not provided");
- }
-
- if (!dynamicLibraries.contains(libraryName)) {
- Owned<DynamicLibrary> dynamicLibrary(new DynamicLibrary());
- Try<Nothing> result = dynamicLibrary->open(libraryName);
- if (!result.isSome()) {
- return Error(
- "Error opening library: '" + libraryName + "': " + result.error());
+ synchronized (mutex) {
+ initialize();
+
+ foreach (const Modules::Library& library, modules.libraries()) {
+ string libraryName;
+ if (library.has_file()) {
+ libraryName = library.file();
+ } else if (library.has_name()) {
+ libraryName = os::libraries::expandName(library.name());
+ } else {
+ return Error("Library name or path not provided");
}
- dynamicLibraries[libraryName] = dynamicLibrary;
- }
-
- // Load module manifests.
- foreach (const Modules::Library::Module& module, library.modules()) {
- if (!module.has_name()) {
- return Error(
- "Error: module name not provided with library '" + libraryName +
- "'");
- }
+ if (!dynamicLibraries.contains(libraryName)) {
+ Owned<DynamicLibrary> dynamicLibrary(new DynamicLibrary());
+ Try<Nothing> result = dynamicLibrary->open(libraryName);
+ if (!result.isSome()) {
+ return Error(
+ "Error opening library: '" + libraryName +
+ "': " + result.error());
+ }
- // Check for possible duplicate module names.
- const std::string moduleName = module.name();
- if (moduleBases.contains(moduleName)) {
- return Error("Error loading duplicate module '" + moduleName + "'");
+ dynamicLibraries[libraryName] = dynamicLibrary;
}
- // Load ModuleBase.
- Try<void*> symbol = dynamicLibraries[libraryName]->loadSymbol(moduleName);
- if (symbol.isError()) {
- return Error(
- "Error loading module '" + moduleName + "': " + symbol.error());
+ // Load module manifests.
+ foreach (const Modules::Library::Module& module, library.modules()) {
+ if (!module.has_name()) {
+ return Error(
+ "Error: module name not provided with library '" + libraryName +
+ "'");
+ }
+
+ // Check for possible duplicate module names.
+ const std::string moduleName = module.name();
+ if (moduleBases.contains(moduleName)) {
+ return Error("Error loading duplicate module '" + moduleName + "'");
+ }
+
+ // Load ModuleBase.
+ Try<void*> symbol =
+ dynamicLibraries[libraryName]->loadSymbol(moduleName);
+ if (symbol.isError()) {
+ return Error(
+ "Error loading module '" + moduleName + "': " + symbol.error());
+ }
+ ModuleBase* moduleBase = (ModuleBase*) symbol.get();
+
+ // Verify module compatibility including version, etc.
+ Try<Nothing> result = verifyModule(moduleName, moduleBase);
+ if (result.isError()) {
+ return Error(
+ "Error verifying module '" + moduleName + "': " + result.error());
+ }
+
+ moduleBases[moduleName] = (ModuleBase*) symbol.get();
+
+ // Now copy the supplied module-specific parameters.
+ moduleParameters[moduleName].mutable_parameter()->CopyFrom(
+ module.parameters());
}
- ModuleBase* moduleBase = (ModuleBase*) symbol.get();
-
- // Verify module compatibility including version, etc.
- Try<Nothing> result = verifyModule(moduleName, moduleBase);
- if (result.isError()) {
- return Error(
- "Error verifying module '" + moduleName + "': " + result.error());
- }
-
- moduleBases[moduleName] = (ModuleBase*) symbol.get();
-
- // Now copy the supplied module-specific parameters.
- moduleParameters[moduleName].mutable_parameter()->CopyFrom(
- module.parameters());
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/98039c99/src/module/manager.hpp
----------------------------------------------------------------------
diff --git a/src/module/manager.hpp b/src/module/manager.hpp
index 4befb64..cab67a8 100644
--- a/src/module/manager.hpp
+++ b/src/module/manager.hpp
@@ -19,9 +19,8 @@
#ifndef __MODULE_MANAGER_HPP__
#define __MODULE_MANAGER_HPP__
-#include <pthread.h>
-
#include <list>
+#include <mutex>
#include <string>
#include <vector>
@@ -37,8 +36,8 @@
#include <stout/check.hpp>
#include <stout/dynamiclibrary.hpp>
#include <stout/hashmap.hpp>
+#include <stout/synchronized.hpp>
-#include "common/lock.hpp"
#include "messages/messages.hpp"
namespace mesos {
@@ -71,40 +70,42 @@ public:
template <typename T>
static Try<T*> create(const std::string& moduleName)
{
- mesos::internal::Lock lock(&mutex);
- if (!moduleBases.contains(moduleName)) {
- return Error(
- "Module '" + moduleName + "' unknown");
- }
+ synchronized (mutex) {
+ if (!moduleBases.contains(moduleName)) {
+ return Error(
+ "Module '" + moduleName + "' unknown");
+ }
- Module<T>* module = (Module<T>*) moduleBases[moduleName];
- if (module->create == NULL) {
- return Error(
- "Error creating module instance for '" + moduleName + "': "
- "create() method not found");
- }
+ Module<T>* module = (Module<T>*) moduleBases[moduleName];
+ if (module->create == NULL) {
+ return Error(
+ "Error creating module instance for '" + moduleName + "': "
+ "create() method not found");
+ }
- std::string expectedKind = kind<T>();
- if (expectedKind != module->kind) {
- return Error(
- "Error creating module instance for '" + moduleName + "': "
- "module is of kind '" + module->kind + "', but the requested "
- "kind is '" + expectedKind + "'");
- }
+ std::string expectedKind = kind<T>();
+ if (expectedKind != module->kind) {
+ return Error(
+ "Error creating module instance for '" + moduleName + "': "
+ "module is of kind '" + module->kind + "', but the requested "
+ "kind is '" + expectedKind + "'");
+ }
- T* instance = module->create(moduleParameters[moduleName]);
- if (instance == NULL) {
- return Error("Error creating Module instance for '" + moduleName + "'");
+ T* instance = module->create(moduleParameters[moduleName]);
+ if (instance == NULL) {
+ return Error("Error creating Module instance for '" + moduleName + "'");
+ }
+ return instance;
}
- return instance;
}
template <typename T>
static bool contains(const std::string& moduleName)
{
- mesos::internal::Lock lock(&mutex);
- return (moduleBases.contains(moduleName) &&
- moduleBases[moduleName]->kind == stringify(kind<T>()));
+ synchronized (mutex) {
+ return (moduleBases.contains(moduleName) &&
+ moduleBases[moduleName]->kind == stringify(kind<T>()));
+ }
}
// Returns all module names that have been loaded that implement the
@@ -117,13 +118,13 @@ public:
template <typename T>
static std::vector<std::string> find()
{
- mesos::internal::Lock lock(&mutex);
-
std::vector<std::string> names;
- foreachpair (const std::string& name, ModuleBase* base, moduleBases) {
- if (base->kind == stringify(kind<T>())) {
- names.push_back(name);
+ synchronized (mutex) {
+ foreachpair (const std::string& name, ModuleBase* base, moduleBases) {
+ if (base->kind == stringify(kind<T>())) {
+ names.push_back(name);
+ }
}
}
@@ -141,9 +142,7 @@ private:
const std::string& moduleName,
const ModuleBase* moduleBase);
- // TODO(karya): Replace pthread_mutex_t with std::mutex in
- // common/lock.hpp and other places that refer to it.
- static pthread_mutex_t mutex;
+ static std::mutex mutex;
static hashmap<const std::string, std::string> kindToVersion;