[hydra-commits] hydra/servicediscovery.git branch "master" updated.

Commits to the Hydra project code repositories hydra-commits at lists.digium.com
Mon Aug 2 07:11:14 CDT 2010


branch "master" has been updated
       via  470dff22c9e44b382ab57a2bcd9657da11b53246 (commit)
       via  6cd49de91000cad218683e5ab8ba717c9d532e0f (commit)
       via  1ffddfb7a5b625ff0f1e8793c24da9457553bf5d (commit)
       via  5693e137a56bdf496279b40606eb5e8b8d53fe9c (commit)
       via  aa56f114821b4a4fcf0839b4cb8950c2f1b7c6b0 (commit)
       via  444f5aa4c17c0ecf6c338a311f53e3a19b8d36b8 (commit)
       via  e08204d9947c4327abd2c8d26873ab25e4ab308d (commit)
       via  70bdc293430e87684f48efc473519676072d97e6 (commit)
      from  e9ca62a4c636b219a9a56d18f88d5e3d1cae7759 (commit)

Summary of changes:
 src/CMakeLists.txt               |    1 +
 src/ServiceLocator.cpp           |    3 +++
 src/ServiceLocatorManagement.cpp |   13 +++++++++++++
 src/ServiceLocatorManagement.h   |    5 +++++
 src/ServiceManagement.cpp        |   15 +++++++++++++++
 src/ServiceManagement.h          |    5 +++++
 6 files changed, 42 insertions(+), 0 deletions(-)


- Log -----------------------------------------------------------------
commit 470dff22c9e44b382ab57a2bcd9657da11b53246
Author: Joshua Colp <jcolp at digium.com>
Date:   Mon Aug 2 09:08:24 2010 -0300

    Remove a lock and add a comment explaining why.

diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index a243be1..4dda3a7 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -135,7 +135,9 @@ void ServiceManagementImpl::unsuspend(const Ice::Current&)
  */
 void ServiceManagementImpl::unregister(const Ice::Current&)
 {
-	boost::unique_lock<boost::shared_mutex> lock(mLock);
+	/* You'll notice no lock here. That's because we aren't actually modifying any internal state that should
+	 * be protected, and if we did lock here there is a chance for a deadlock which is super sad.
+	 */
 
 	mAdapter->remove(mManagementPrx->ice_getIdentity());
 

commit 6cd49de91000cad218683e5ab8ba717c9d532e0f
Author: Joshua Colp <jcolp at digium.com>
Date:   Sun Aug 1 18:12:27 2010 -0300

    Add a missing lock.

diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index 64e4161..a243be1 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -135,6 +135,8 @@ void ServiceManagementImpl::unsuspend(const Ice::Current&)
  */
 void ServiceManagementImpl::unregister(const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
+
 	mAdapter->remove(mManagementPrx->ice_getIdentity());
 
 	mManagement->removeService(this);

commit 1ffddfb7a5b625ff0f1e8793c24da9457553bf5d
Merge: 5693e13 e9ca62a
Author: Joshua Colp <jcolp at digium.com>
Date:   Sat Jul 31 20:17:06 2010 -0300

    Merge branch 'master' into locking

diff --cc src/ServiceLocatorManagement.cpp
index e10275e,0dce45b..3c31c67
--- a/src/ServiceLocatorManagement.cpp
+++ b/src/ServiceLocatorManagement.cpp
@@@ -17,11 -17,7 +17,10 @@@
   */
  
  #include <Ice/Ice.h>
- #include <IceStorm/IceStorm.h>
  
 +#include <boost/thread/thread.hpp>
 +#include <boost/thread/shared_mutex.hpp>
 +
  #include "service_locator.h"
  #include "service_locator_events.h"
  
diff --cc src/ServiceManagement.cpp
index 4a2a68c,214fa87..64e4161
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@@ -17,11 -17,7 +17,10 @@@
   */
  
  #include <Ice/Ice.h>
- #include <IceStorm/IceStorm.h>
  
 +#include <boost/thread/thread.hpp>
 +#include <boost/thread/shared_mutex.hpp>
 +
  #include "service_locator.h"
  #include "service_locator_events.h"
  

commit 5693e137a56bdf496279b40606eb5e8b8d53fe9c
Author: Joshua Colp <jcolp at digium.com>
Date:   Sat Jul 31 20:06:40 2010 -0300

    Add locking to protect ServiceManagementImpl.

diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index 4ba11a2..4a2a68c 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -55,6 +55,8 @@ ServiceManagementImpl::ServiceManagementImpl(ServiceLocatorManagementImpl* manag
  */
 bool ServiceManagementImpl::isSupported(const ServiceLocatorParamsPtr& params)
 {
+	boost::shared_lock<boost::shared_mutex> lock(mLock);
+
 	/* If this service is suspended we can just skip the entire check and return false now, easy as pie */
 	if (mSuspended) {
 		return false;
@@ -98,6 +100,8 @@ bool ServiceLocatorParamsSpec::isSupported(const ServiceLocatorParamsPtr& params
  */
 void ServiceManagementImpl::addLocatorParams(const ServiceLocatorParamsPtr& params, const std::string& compare_guid, const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
+
 	ServiceLocatorParamsSpec spec(params, compare_guid, mManagement);
 
 	mSupportedLocatorParams.push_back(spec);
@@ -108,6 +112,8 @@ void ServiceManagementImpl::addLocatorParams(const ServiceLocatorParamsPtr& para
  */
 void ServiceManagementImpl::suspend(const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
+
 	mSuspended = true;
 
 	mLocatorTopic->serviceSuspended(mGuid);
@@ -118,6 +124,8 @@ void ServiceManagementImpl::suspend(const Ice::Current&)
  */
 void ServiceManagementImpl::unsuspend(const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
+
 	mSuspended = false;
 
 	mLocatorTopic->serviceUnsuspended(mGuid);
diff --git a/src/ServiceManagement.h b/src/ServiceManagement.h
index 237e5ec..76ede8a 100644
--- a/src/ServiceManagement.h
+++ b/src/ServiceManagement.h
@@ -71,6 +71,11 @@ public:
 	bool isSupported(const Hydra::Location::ServiceLocatorParamsPtr&);
 private:
 	/**
+	 * Shared mutex lock which protects the service.
+	 */
+	boost::shared_mutex mLock;
+
+	/**
 	 * A boolean value which is used to store the suspended state of this service.
 	 */
 	bool mSuspended;

commit aa56f114821b4a4fcf0839b4cb8950c2f1b7c6b0
Author: Joshua Colp <jcolp at digium.com>
Date:   Sat Jul 31 20:04:50 2010 -0300

    Fix a deadlock due to a second lock attempt.

diff --git a/src/ServiceLocatorManagement.cpp b/src/ServiceLocatorManagement.cpp
index cef1dc7..e10275e 100644
--- a/src/ServiceLocatorManagement.cpp
+++ b/src/ServiceLocatorManagement.cpp
@@ -119,7 +119,9 @@ void ServiceLocatorManagementImpl::removeCompare(const string& guid, const Ice::
  */
 bool ServiceLocatorManagementImpl::isSupported(const string& compare_guid, const ServiceLocatorParamsPtr& params)
 {
-	boost::unique_lock<boost::shared_mutex> lock(mLock);
+	/* You'll note there is no lock here. This is because we already have a lock in the locate or locateAll
+	 * functions.
+	 */
 	map<string, ServiceLocatorComparator>::iterator comparator = mCompares.find(compare_guid);
 
 	if (comparator == mCompares.end()) {

commit 444f5aa4c17c0ecf6c338a311f53e3a19b8d36b8
Author: Joshua Colp <jcolp at digium.com>
Date:   Sat Jul 31 20:01:37 2010 -0300

    Expand the comment about the lock a bit.

diff --git a/src/ServiceLocatorManagement.h b/src/ServiceLocatorManagement.h
index 7e4131d..e9cc2f6 100644
--- a/src/ServiceLocatorManagement.h
+++ b/src/ServiceLocatorManagement.h
@@ -75,7 +75,7 @@ ServiceLocatorManagementImpl(Ice::ObjectAdapterPtr adapter, const Hydra::Locatio
 	void removeService(ServiceManagementImplPtr);
 private:
 	/**
-	 * Shared mutex lock which protects the services
+	 * Shared mutex lock which protects the services and comparators.
 	 */
 	boost::shared_mutex mLock;
 

commit e08204d9947c4327abd2c8d26873ab25e4ab308d
Author: Joshua Colp <jcolp at digium.com>
Date:   Sat Jul 31 20:00:47 2010 -0300

    Add shared mutex locking at the ServiceLocatorManagement level. Next step is to add it to the
    ServiceManagementImpl level.

diff --git a/src/ServiceLocator.cpp b/src/ServiceLocator.cpp
index 41925c1..6b6951c 100644
--- a/src/ServiceLocator.cpp
+++ b/src/ServiceLocator.cpp
@@ -19,6 +19,9 @@
 #include <Ice/Ice.h>
 #include <IceStorm/IceStorm.h>
 
+#include <boost/thread/thread.hpp>
+#include <boost/thread/shared_mutex.hpp>
+
 #include "service_locator.h"
 #include "service_locator_events.h"
 
diff --git a/src/ServiceLocatorManagement.cpp b/src/ServiceLocatorManagement.cpp
index 7fe47f3..cef1dc7 100644
--- a/src/ServiceLocatorManagement.cpp
+++ b/src/ServiceLocatorManagement.cpp
@@ -19,6 +19,9 @@
 #include <Ice/Ice.h>
 #include <IceStorm/IceStorm.h>
 
+#include <boost/thread/thread.hpp>
+#include <boost/thread/shared_mutex.hpp>
+
 #include "service_locator.h"
 #include "service_locator_events.h"
 
@@ -33,6 +36,8 @@ using namespace Hydra::Location;
  */
 Ice::ObjectPrx ServiceLocatorManagementImpl::locate(const ServiceLocatorParamsPtr& params)
 {
+	boost::shared_lock<boost::shared_mutex> lock(mLock);
+
 	for (vector<ServiceManagementImplPtr>::iterator service = mServices.begin(); service != mServices.end(); ++service) {
 		if ((*service)->isSupported(params) == true) {
 			return (*service)->GetService();
@@ -47,6 +52,7 @@ Ice::ObjectPrx ServiceLocatorManagementImpl::locate(const ServiceLocatorParamsPt
  */
 Ice::ObjectProxySeq ServiceLocatorManagementImpl::locateAll(const ServiceLocatorParamsPtr& params)
 {
+	boost::shared_lock<boost::shared_mutex> lock(mLock);
 	Ice::ObjectProxySeq applicable_services;
 
 	for (vector<ServiceManagementImplPtr>::iterator service = mServices.begin(); service != mServices.end(); ++service) {
@@ -67,6 +73,7 @@ Ice::ObjectProxySeq ServiceLocatorManagementImpl::locateAll(const ServiceLocator
  */
 ServiceManagementPrx ServiceLocatorManagementImpl::addService(const Ice::ObjectPrx& service, const string& guid, const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
 	ServiceManagementImplPtr new_service = new ServiceManagementImpl(this, service, mAdapter, mLocatorTopic, guid);
 
 	mServices.push_back(new_service);
@@ -79,6 +86,7 @@ ServiceManagementPrx ServiceLocatorManagementImpl::addService(const Ice::ObjectP
  */
 void ServiceLocatorManagementImpl::addCompare(const string& guid, const ServiceLocatorParamsComparePrx& service, const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
 	ServiceLocatorComparator new_comparator(service);
 
 	pair<map<string, ServiceLocatorComparator>::iterator, bool> insert_pair;
@@ -96,6 +104,7 @@ void ServiceLocatorManagementImpl::addCompare(const string& guid, const ServiceL
  */
 void ServiceLocatorManagementImpl::removeCompare(const string& guid, const Ice::Current&)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
 	int erased = mCompares.erase(guid);
 
 	if (!erased) {
@@ -110,6 +119,7 @@ void ServiceLocatorManagementImpl::removeCompare(const string& guid, const Ice::
  */
 bool ServiceLocatorManagementImpl::isSupported(const string& compare_guid, const ServiceLocatorParamsPtr& params)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
 	map<string, ServiceLocatorComparator>::iterator comparator = mCompares.find(compare_guid);
 
 	if (comparator == mCompares.end()) {
@@ -124,6 +134,7 @@ bool ServiceLocatorManagementImpl::isSupported(const string& compare_guid, const
  */
 void ServiceLocatorManagementImpl::removeService(ServiceManagementImplPtr service)
 {
+	boost::unique_lock<boost::shared_mutex> lock(mLock);
 	for (vector<ServiceManagementImplPtr>::iterator existing_service = mServices.begin(); existing_service != mServices.end(); ++existing_service) {
 		if ((*existing_service) == service) {
 			mServices.erase(existing_service);
diff --git a/src/ServiceLocatorManagement.h b/src/ServiceLocatorManagement.h
index 69fe736..7e4131d 100644
--- a/src/ServiceLocatorManagement.h
+++ b/src/ServiceLocatorManagement.h
@@ -75,6 +75,11 @@ ServiceLocatorManagementImpl(Ice::ObjectAdapterPtr adapter, const Hydra::Locatio
 	void removeService(ServiceManagementImplPtr);
 private:
 	/**
+	 * Shared mutex lock which protects the services
+	 */
+	boost::shared_mutex mLock;
+
+	/**
 	 * Object adapter that our service management proxies originate from, it is houses the
 	 * main management service.
 	 */
diff --git a/src/ServiceManagement.cpp b/src/ServiceManagement.cpp
index c6862d7..4ba11a2 100644
--- a/src/ServiceManagement.cpp
+++ b/src/ServiceManagement.cpp
@@ -19,6 +19,9 @@
 #include <Ice/Ice.h>
 #include <IceStorm/IceStorm.h>
 
+#include <boost/thread/thread.hpp>
+#include <boost/thread/shared_mutex.hpp>
+
 #include "service_locator.h"
 #include "service_locator_events.h"
 

commit 70bdc293430e87684f48efc473519676072d97e6
Author: Joshua Colp <jcolp at digium.com>
Date:   Sat Jul 31 19:46:49 2010 -0300

    Make ourselves depend on boost's thread capability so we can use shared_mutex.

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index ea5d80e..ee51c0b 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -6,5 +6,6 @@ hydra_component_add_file(service_locator ServiceLocator.cpp)
 hydra_component_add_file(service_locator ServiceLocatorManagement.cpp)
 hydra_component_add_file(service_locator ServiceManagement.cpp)
 hydra_component_add_ice_libraries(service_locator IceStorm)
+hydra_component_add_boost_libraries(service_locator thread)
 hydra_component_build_standalone(service_locator)
 hydra_component_install(service_locator RUNTIME bin "Service Locator." Core)

-----------------------------------------------------------------------


-- 
hydra/servicediscovery.git




More information about the asterisk-scf-commits mailing list