summaryrefslogtreecommitdiff
path: root/cpp
diff options
context:
space:
mode:
Diffstat (limited to 'cpp')
-rw-r--r--cpp/src/Ice/LocatorInfo.cpp115
-rw-r--r--cpp/src/Ice/LocatorInfo.h9
-rw-r--r--cpp/src/Ice/Proxy.cpp77
-rw-r--r--cpp/src/Ice/RouterInfo.cpp4
-rw-r--r--cpp/src/Ice/RouterInfo.h4
5 files changed, 96 insertions, 113 deletions
diff --git a/cpp/src/Ice/LocatorInfo.cpp b/cpp/src/Ice/LocatorInfo.cpp
index 8d24f78e938..e57a3bbef9b 100644
--- a/cpp/src/Ice/LocatorInfo.cpp
+++ b/cpp/src/Ice/LocatorInfo.cpp
@@ -133,13 +133,22 @@ IceInternal::LocatorAdapterTable::add(const string& adapter, const ::std::vector
_adapterEndpointsMap.insert(make_pair(adapter, endpoints));
}
-void
+::std::vector<EndpointPtr>
IceInternal::LocatorAdapterTable::remove(const string& adapter)
{
IceUtil::Mutex::Lock sync(*this);
- bool removed = _adapterEndpointsMap.erase(adapter) > 0;
- assert(removed);
+ std::map<std::string, std::vector<EndpointPtr> >::iterator p = _adapterEndpointsMap.find(adapter);
+ if(p == _adapterEndpointsMap.end())
+ {
+ return std::vector<EndpointPtr>();
+ }
+
+ std::vector<EndpointPtr> endpoints = p->second;
+
+ _adapterEndpointsMap.erase(p);
+
+ return endpoints;
}
@@ -169,12 +178,8 @@ IceInternal::LocatorInfo::operator<(const LocatorInfo& rhs) const
return _locator < rhs._locator;
}
-// TODO: ML: Should be const. I know that I didn't do this correctly
-// for other classes (such as RouterInfo) as well. However, we need to
-// start doing this right :-) Can you please fix const-correctness for the
-// locator and router stuff?
LocatorPrx
-IceInternal::LocatorInfo::getLocator()
+IceInternal::LocatorInfo::getLocator() const
{
//
// No mutex lock necessary, _locator is immutable.
@@ -195,15 +200,19 @@ IceInternal::LocatorInfo::getLocatorRegistry()
return _locatorRegistry;
}
-bool
-IceInternal::LocatorInfo::getEndpoints(const ReferencePtr& ref, ::std::vector<EndpointPtr>& endpoints) const
+std::vector<EndpointPtr>
+IceInternal::LocatorInfo::getEndpoints(const ReferencePtr& ref, bool& cached)
{
- bool cached = false;
+ ::std::vector<EndpointPtr> endpoints;
if(!ref->adapterId.empty())
{
+ cached = true;
+
if(!_adapterTable->get(ref->adapterId, endpoints))
{
+ cached = false;
+
//
// Search the adapter in the location service if we didn't
// find it in the cache.
@@ -223,36 +232,29 @@ IceInternal::LocatorInfo::getEndpoints(const ReferencePtr& ref, ::std::vector<En
// endpoints and raise a NoEndpointException().
//
}
- }
- else
- {
- cached = true;
+
+ //
+ // Add to the cache.
+ //
+ if(!endpoints.empty())
+ {
+ _adapterTable->add(ref->adapterId, endpoints);
+ }
}
- // TODO: ML: Don't use size to check for emptyness. Use !endpoints.empty().
- if(endpoints.size() > 0)
+ if(!endpoints.empty())
{
if(ref->instance->traceLevels()->location >= 1)
{
- // TODO: ML: "object"? You mean "proxy"! Or even
- // better, since only the endpoints count here, use
- // "endpoints" instead of "object".
-
Trace out(ref->instance->logger(), ref->instance->traceLevels()->locationCat);
if(cached)
{
- // TODO: ML: "local locator table"? Isn't this always
- // local? I suggest to remove the "local".
- out << "found object in local locator table\n";
+ out << "found endpoints in locator table\n";
}
else
{
- // TODO: ML: "found"? Shouldn't this read
- // "retrieved endpoints from locator".
- out << "found object in locator\n";
+ out << "retrieved endpoints from locator, adding to locator table\n";
}
- // TODO: ML: Why do you log the identity? What meaning does it have?
- out << "identity = " << identityToString(ref->identity) << "\n";
out << "adapter = " << ref->adapterId << "\n";
const char* sep = endpoints.size() > 1 ? ":" : "";
ostringstream o;
@@ -262,50 +264,33 @@ IceInternal::LocatorInfo::getEndpoints(const ReferencePtr& ref, ::std::vector<En
}
}
}
-
- return cached;
-}
-
-// TODO: ML: Why a separate operation? Why not add this stuff to
-// getEndpoings() above? What's the point of getting endpoints without
-// caching them?
-void
-IceInternal::LocatorInfo::addEndpoints(const ReferencePtr& ref, const ::std::vector<EndpointPtr>& endpoints)
-{
- assert(!endpoints.empty());
-
- if(!ref->adapterId.empty())
+ else
{
- _adapterTable->add(ref->adapterId, endpoints);
-
- if(ref->instance->traceLevels()->location >= 2)
- {
- Trace out(ref->instance->logger(), ref->instance->traceLevels()->locationCat);
- // TODO: Since this function should be removed, no
- // separate log message is necessary either. Instead use
- // "retrieved endpoints from locator, adding to locator
- // table" above.
- out << "added adapter to local locator table\n";
- out << "adapter = " << ref->adapterId;
- }
+ cached = false;
}
+
+ return endpoints;
}
-// TODO: ML: Should be named to "clearCache".
void
-IceInternal::LocatorInfo::removeEndpoints(const ReferencePtr& ref)
+IceInternal::LocatorInfo::clearCache(const ReferencePtr& ref)
{
if(!ref->adapterId.empty())
{
- _adapterTable->remove(ref->adapterId);
-
- if(ref->instance->traceLevels()->location >= 2)
+ std::vector<EndpointPtr> endpoints = _adapterTable->remove(ref->adapterId);
+ if(!endpoints.empty())
{
- Trace out(ref->instance->logger(), ref->instance->traceLevels()->locationCat);
- // TODO: ML: "removed endpoints from locator table"
- out << "removed adapter from local locator table\n";
- out << "adapter = " << ref->adapterId;
- // TODO: ML: Log endpoints, like in getEndpoints().
- }
+ if(ref->instance->traceLevels()->location >= 1)
+ {
+ Trace out(ref->instance->logger(), ref->instance->traceLevels()->locationCat);
+ out << "removed endpoints from locator table\n";
+ out << "adapter = " << ref->adapterId;
+ const char* sep = endpoints.size() > 1 ? ":" : "";
+ ostringstream o;
+ transform(endpoints.begin(), endpoints.end(), ostream_iterator<string>(o, sep),
+ ::Ice::constMemFun(&Endpoint::toString));
+ out << "endpoints = " << o.str();
+ }
+ }
}
}
diff --git a/cpp/src/Ice/LocatorInfo.h b/cpp/src/Ice/LocatorInfo.h
index 9bee858982f..2efeb642102 100644
--- a/cpp/src/Ice/LocatorInfo.h
+++ b/cpp/src/Ice/LocatorInfo.h
@@ -50,7 +50,7 @@ public:
bool get(const std::string&, ::std::vector<EndpointPtr>&) const;
void add(const std::string&, const ::std::vector<EndpointPtr>&);
- void remove(const std::string&);
+ ::std::vector<EndpointPtr> remove(const std::string&);
private:
@@ -67,12 +67,11 @@ public:
bool operator!=(const LocatorInfo&) const;
bool operator<(const LocatorInfo&) const;
- ::Ice::LocatorPrx getLocator();
+ ::Ice::LocatorPrx getLocator() const;
::Ice::LocatorRegistryPrx getLocatorRegistry();
- bool getEndpoints(const ReferencePtr&, ::std::vector<EndpointPtr>&) const;
- void addEndpoints(const ReferencePtr&, const ::std::vector<EndpointPtr>&);
- void removeEndpoints(const ReferencePtr&);
+ std::vector<EndpointPtr> getEndpoints(const ReferencePtr&, bool&);
+ void clearCache(const ReferencePtr&);
private:
diff --git a/cpp/src/Ice/Proxy.cpp b/cpp/src/Ice/Proxy.cpp
index b99a8f0c61e..2dda96c4062 100644
--- a/cpp/src/Ice/Proxy.cpp
+++ b/cpp/src/Ice/Proxy.cpp
@@ -531,9 +531,13 @@ IceProxy::Ice::Object::__copyFrom(const ObjectPrx& from)
void
IceProxy::Ice::Object::__handleException(const LocalException& ex, int& cnt)
{
- IceUtil::Mutex::Lock sync(*this);
-
- _delegate = 0;
+ //
+ // Only _delegate needs to be mutex protected here.
+ //
+ {
+ IceUtil::Mutex::Lock sync(*this);
+ _delegate = 0;
+ }
try
{
@@ -562,14 +566,13 @@ IceProxy::Ice::Object::__handleException(const LocalException& ex, int& cnt)
{
++cnt;
}
-
+
TraceLevelsPtr traceLevels = _reference->instance->traceLevels();
LoggerPtr logger = _reference->instance->logger();
const std::vector<int>& retryIntervals = _reference->instance->proxyFactory()->getRetryIntervals();
-
- // TODO: ML: Loose C-style cast.
- if(cnt > (int)retryIntervals.size())
+
+ if(cnt > static_cast<int>(retryIntervals.size()))
{
if(traceLevels->retry >= 1)
{
@@ -593,8 +596,7 @@ IceProxy::Ice::Object::__handleException(const LocalException& ex, int& cnt)
if(cnt > 0)
{
//
- // Sleep before retrying. TODO: is it safe to sleep here
- // with the mutex locked? TODO: ML: Why not sleep outside the mutex lock?</ml>
+ // Sleep before retrying.
//
IceUtil::ThreadControl::sleep(IceUtil::Time::milliSeconds(retryIntervals[cnt - 1]));
}
@@ -612,6 +614,11 @@ IceProxy::Ice::Object::__locationForward(const LocationForward& ex)
throw LocationForwardIdentityException(__FILE__, __LINE__);
}
+ //
+ // TODO: BENOIT: This is not thread-safe. Everywhere else in the
+ // code, _reference is considered immutable and is not mutex
+ // protected.
+ //
_reference = _reference->changeAdapterId(ex._prx->_reference->adapterId);
_reference = _reference->changeEndpoints(ex._prx->_reference->endpoints);
@@ -872,11 +879,12 @@ IceDelegateM::Ice::Object::setup(const ReferencePtr& ref)
__connection->incProxyUsageCount();
}
else
- {
+ {
while(true)
{
- vector<EndpointPtr> endpoints;
- bool cached = false;
+ bool cached;
+ vector<EndpointPtr> endpoints;
+
if(__reference->routerInfo)
{
//
@@ -892,12 +900,7 @@ IceDelegateM::Ice::Object::setup(const ReferencePtr& ref)
}
else if(__reference->locatorInfo)
{
- //
- // TODO: ML: This call is confusing. It should be:
- // endpoints = __reference->locatorInfo->getEndpoints(__reference, cached);
- // Please change the signature accordingly.
- //
- cached = __reference->locatorInfo->getEndpoints(__reference, endpoints);
+ endpoints = __reference->locatorInfo->getEndpoints(__reference, cached);
}
vector<EndpointPtr> filteredEndpoints = filterEndpoints(endpoints);
@@ -915,32 +918,28 @@ IceDelegateM::Ice::Object::setup(const ReferencePtr& ref)
}
catch(const LocalException& ex)
{
- if(cached)
- {
- TraceLevelsPtr traceLevels = __reference->instance->traceLevels();
- LoggerPtr logger = __reference->instance->logger();
+ if(!__reference->routerInfo && __reference->endpoints.empty())
+ {
+ assert(__reference->locatorInfo);
+ __reference->locatorInfo->clearCache(__reference);
- if(traceLevels->retry >= 2)
+ if(cached)
{
- Trace out(logger, traceLevels->retryCat);
- out << "connection to cached endpoints failed\n"
- << "removing endpoints from cache and trying one more time\n" << ex;
+ TraceLevelsPtr traceLevels = __reference->instance->traceLevels();
+ LoggerPtr logger = __reference->instance->logger();
+ if(traceLevels->retry >= 2)
+ {
+ Trace out(logger, traceLevels->retryCat);
+ out << "connection to cached endpoints failed\n"
+ << "removing endpoints from cache and trying one more time\n" << ex;
+ }
+ continue;
}
-
- assert(__reference->locatorInfo);
- __reference->locatorInfo->removeEndpoints(__reference);
- continue;
- }
- else
- {
- throw;
}
- }
-
- if(__reference->locatorInfo && !cached)
- {
- __reference->locatorInfo->addEndpoints(__reference, endpoints);
+
+ throw;
}
+
break;
}
diff --git a/cpp/src/Ice/RouterInfo.cpp b/cpp/src/Ice/RouterInfo.cpp
index 412032f4dc9..a36b7d27234 100644
--- a/cpp/src/Ice/RouterInfo.cpp
+++ b/cpp/src/Ice/RouterInfo.cpp
@@ -102,7 +102,7 @@ IceInternal::RouterInfo::operator<(const RouterInfo& rhs) const
}
RouterPrx
-IceInternal::RouterInfo::getRouter()
+IceInternal::RouterInfo::getRouter() const
{
//
// No mutex lock necessary, _router is immutable.
@@ -181,7 +181,7 @@ IceInternal::RouterInfo::setAdapter(const ObjectAdapterPtr& adapter)
}
ObjectAdapterPtr
-IceInternal::RouterInfo::getAdapter()
+IceInternal::RouterInfo::getAdapter() const
{
IceUtil::Mutex::Lock sync(*this);
return _adapter;
diff --git a/cpp/src/Ice/RouterInfo.h b/cpp/src/Ice/RouterInfo.h
index 15fa489175e..8223cd5012c 100644
--- a/cpp/src/Ice/RouterInfo.h
+++ b/cpp/src/Ice/RouterInfo.h
@@ -51,14 +51,14 @@ public:
bool operator!=(const RouterInfo&) const;
bool operator<(const RouterInfo&) const;
- ::Ice::RouterPrx getRouter();
+ ::Ice::RouterPrx getRouter() const;
::Ice::ObjectPrx getClientProxy();
void setClientProxy(const ::Ice::ObjectPrx&);
::Ice::ObjectPrx getServerProxy();
void setServerProxy(const ::Ice::ObjectPrx&);
void addProxy(const ::Ice::ObjectPrx&);
void setAdapter(const ::Ice::ObjectAdapterPtr&);
- ::Ice::ObjectAdapterPtr getAdapter();
+ ::Ice::ObjectAdapterPtr getAdapter() const;
private:
generated by cgit v1.2.3 (git 2.46.0) at 2025-08-03 20:17:23 +0000