diff options
author | Kenneth Anthony Giusti <kgiusti@apache.org> | 2012-12-06 20:39:26 +0000 |
---|---|---|
committer | Kenneth Anthony Giusti <kgiusti@apache.org> | 2012-12-06 20:39:26 +0000 |
commit | d22a5a95b072d6e5aa115b26727563b7fc921de7 (patch) | |
tree | de71555fccf7217f5244ca70c6502b871eb589ae | |
parent | 6d565ecf62d258bd3a52d4d9bc89a9691003aeb6 (diff) | |
download | qpid-python-d22a5a95b072d6e5aa115b26727563b7fc921de7.tar.gz |
QPID-4485: hold agentLock and check for dups when adding new mgmt objects
(cherry picked from commit bd186e2480bc1999e9c9921cb7bd5710ddcf5128)
git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/0.20@1418065 13f79535-47bb-0310-9956-ffa450edef68
-rw-r--r-- | qpid/cpp/src/qpid/agent/ManagementAgentImpl.cpp | 42 | ||||
-rw-r--r-- | qpid/cpp/src/qpid/agent/ManagementAgentImpl.h | 2 |
2 files changed, 35 insertions, 9 deletions
diff --git a/qpid/cpp/src/qpid/agent/ManagementAgentImpl.cpp b/qpid/cpp/src/qpid/agent/ManagementAgentImpl.cpp index 09b7fa58e9..a48789973a 100644 --- a/qpid/cpp/src/qpid/agent/ManagementAgentImpl.cpp +++ b/qpid/cpp/src/qpid/agent/ManagementAgentImpl.cpp @@ -654,7 +654,10 @@ void ManagementAgentImpl::invokeMethodRequest(const string& body, const string& void ManagementAgentImpl::handleGetQuery(const string& body, const string& cid, const string& rte, const string& rtk) { - moveNewObjectsLH(); + { + sys::Mutex::ScopedLock lock(agentLock); + moveNewObjectsLH(lock); + } Variant::Map inMap; Variant::Map::const_iterator i; @@ -985,14 +988,37 @@ ManagementAgentImpl::PackageMap::iterator ManagementAgentImpl::findOrAddPackage( return result.first; } -void ManagementAgentImpl::moveNewObjectsLH() +// note well: caller must hold agentLock when calling this! +void ManagementAgentImpl::moveNewObjectsLH(const sys::Mutex::ScopedLock& /*agentLock*/) { sys::Mutex::ScopedLock lock(addLock); - for (ObjectMap::iterator iter = newManagementObjects.begin(); - iter != newManagementObjects.end(); - iter++) - managementObjects[iter->first] = iter->second; - newManagementObjects.clear(); + ObjectMap::iterator newObj = newManagementObjects.begin(); + while (newObj != newManagementObjects.end()) { + // before adding a new mgmt object, check for duplicates: + ObjectMap::iterator oldObj = managementObjects.find(newObj->first); + if (oldObj == managementObjects.end()) { + managementObjects[newObj->first] = newObj->second; + newManagementObjects.erase(newObj++); // post inc iterator safe! + } else { + // object exists with same object id. This may be legit, for example, when a + // recently deleted object is re-added before the mgmt poll runs. + if (newObj->second->isDeleted()) { + // @TODO fixme: we missed an add-delete for the new object + QPID_LOG(warning, "Mgmt Object deleted before update sent, oid=" << newObj->first); + newManagementObjects.erase(newObj++); // post inc iterator safe! + } else if (oldObj->second->isDeleted()) { + // skip adding newObj, try again later once oldObj has been cleaned up by poll + ++newObj; + } else { + // real bad - two objects exist with same OID. This is a bug in the application + QPID_LOG(error, "Detected two Mgmt Objects using the same object id! oid=" << newObj->first + << ", this is bad!"); + // what to do here? Can't erase an active obj - owner has a pointer to it. + // for now I punt. Maybe the flood of log messages will get someone's attention :P + ++newObj; + } + } + } } void ManagementAgentImpl::addClassLocal(uint8_t classKind, @@ -1060,7 +1086,7 @@ void ManagementAgentImpl::periodicProcessing() if (!connected) return; - moveNewObjectsLH(); + moveNewObjectsLH(lock); // // Clear the been-here flag on all objects in the map. diff --git a/qpid/cpp/src/qpid/agent/ManagementAgentImpl.h b/qpid/cpp/src/qpid/agent/ManagementAgentImpl.h index 53f3c13a91..d801989f64 100644 --- a/qpid/cpp/src/qpid/agent/ManagementAgentImpl.h +++ b/qpid/cpp/src/qpid/agent/ManagementAgentImpl.h @@ -261,7 +261,7 @@ class ManagementAgentImpl : public ManagementAgent, public client::MessageListen void storeData(bool requested=false); void retrieveData(std::string& vendor, std::string& product, std::string& inst); PackageMap::iterator findOrAddPackage(const std::string& name); - void moveNewObjectsLH(); + void moveNewObjectsLH(const sys::Mutex::ScopedLock& agentLock); void addClassLocal (uint8_t classKind, PackageMap::iterator pIter, const std::string& className, |