From d3b3dfd76d14b82f11596f677b0be3e072399ac5 Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 22 Feb 2019 18:05:42 -0800 Subject: [PATCH 1/2] Don't send a KillAvatar packet on kick The DS already takes cares of removing nodes no longer allowed when the permissions are updated and KillAvatar was never meant to be used from the DS. --- domain-server/src/DomainServerSettingsManager.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index 780fad15f2..4e833f6b77 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -25,13 +25,13 @@ #include #include +#include #include #include #include #include #include #include -#include //for KillAvatarReason #include #include "DomainServerNodeData.h" @@ -870,14 +870,6 @@ void DomainServerSettingsManager::processNodeKickRequestPacket(QSharedPointerwrite(nodeUUID.toRfc4122()); - packet->writePrimitive(KillAvatarReason::NoReason); - - // send to avatar mixer, it sends the kill to everyone else - limitedNodeList->broadcastToNodes(std::move(packet), NodeSet() << NodeType::AvatarMixer); - if (newPermissions) { qDebug() << "Removing connect permission for node" << uuidStringWithoutCurlyBraces(matchingNode->getUUID()) << "after kick request from" << uuidStringWithoutCurlyBraces(sendingNode->getUUID()); From 50035a198366ad46ffc61fd42d2847302f501b6b Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 22 Feb 2019 18:07:26 -0800 Subject: [PATCH 2/2] Delete old nodes with identical socket --- libraries/networking/src/LimitedNodeList.cpp | 176 +++++++++---------- 1 file changed, 84 insertions(+), 92 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 8b9e37569c..eaa02f059e 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -645,103 +645,95 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, Node::LocalID localID, bool isReplicated, bool isUpstream, const QUuid& connectionSecret, const NodePermissions& permissions) { - QReadLocker readLocker(&_nodeMutex); - NodeHash::const_iterator it = _nodeHash.find(uuid); + { + QReadLocker readLocker(&_nodeMutex); + NodeHash::const_iterator it = _nodeHash.find(uuid); - if (it != _nodeHash.end()) { - SharedNodePointer& matchingNode = it->second; + if (it != _nodeHash.end()) { + SharedNodePointer& matchingNode = it->second; - matchingNode->setPublicSocket(publicSocket); - matchingNode->setLocalSocket(localSocket); - matchingNode->setPermissions(permissions); - matchingNode->setConnectionSecret(connectionSecret); - matchingNode->setIsReplicated(isReplicated); - matchingNode->setIsUpstream(isUpstream || NodeType::isUpstream(nodeType)); - matchingNode->setLocalID(localID); + matchingNode->setPublicSocket(publicSocket); + matchingNode->setLocalSocket(localSocket); + matchingNode->setPermissions(permissions); + matchingNode->setConnectionSecret(connectionSecret); + matchingNode->setIsReplicated(isReplicated); + matchingNode->setIsUpstream(isUpstream || NodeType::isUpstream(nodeType)); + matchingNode->setLocalID(localID); - return matchingNode; - } else { - auto it = _connectionIDs.find(uuid); - if (it == _connectionIDs.end()) { - _connectionIDs[uuid] = INITIAL_CONNECTION_ID; + return matchingNode; } - - // we didn't have this node, so add them - Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket); - newNode->setIsReplicated(isReplicated); - newNode->setIsUpstream(isUpstream || NodeType::isUpstream(nodeType)); - newNode->setConnectionSecret(connectionSecret); - newNode->setPermissions(permissions); - newNode->setLocalID(localID); - - // move the newly constructed node to the LNL thread - newNode->moveToThread(thread()); - - if (nodeType == NodeType::AudioMixer) { - LimitedNodeList::flagTimeForConnectionStep(LimitedNodeList::AddedAudioMixer); - } - - SharedNodePointer newNodePointer(newNode, &QObject::deleteLater); - - // if this is a solo node type, we assume that the DS has replaced its assignment and we should kill the previous node - if (SOLO_NODE_TYPES.count(newNode->getType())) { - // while we still have the read lock, see if there is a previous solo node we'll need to remove - auto previousSoloIt = std::find_if(_nodeHash.cbegin(), _nodeHash.cend(), [newNode](const UUIDNodePair& nodePair){ - return nodePair.second->getType() == newNode->getType(); - }); - - if (previousSoloIt != _nodeHash.cend()) { - // we have a previous solo node, switch to a write lock so we can remove it - readLocker.unlock(); - - QWriteLocker writeLocker(&_nodeMutex); - - auto oldSoloNode = previousSoloIt->second; - - _localIDMap.unsafe_erase(oldSoloNode->getLocalID()); - _nodeHash.unsafe_erase(previousSoloIt); - handleNodeKill(oldSoloNode); - - // convert the current lock back to a read lock for insertion of new node - writeLocker.unlock(); - readLocker.relock(); - } - } - - // insert the new node and release our read lock -#if defined(Q_OS_ANDROID) || (defined(__clang__) && defined(Q_OS_LINUX)) - _nodeHash.insert(UUIDNodePair(newNode->getUUID(), newNodePointer)); - _localIDMap.insert(std::pair(localID, newNodePointer)); -#else - _nodeHash.emplace(newNode->getUUID(), newNodePointer); - _localIDMap.emplace(localID, newNodePointer); -#endif - readLocker.unlock(); - - qCDebug(networking) << "Added" << *newNode; - - auto weakPtr = newNodePointer.toWeakRef(); // We don't want the lambdas to hold a strong ref - - emit nodeAdded(newNodePointer); - if (newNodePointer->getActiveSocket()) { - emit nodeActivated(newNodePointer); - } else { - connect(newNodePointer.data(), &NetworkPeer::socketActivated, this, [this, weakPtr] { - auto sharedPtr = weakPtr.lock(); - if (sharedPtr) { - emit nodeActivated(sharedPtr); - disconnect(sharedPtr.data(), &NetworkPeer::socketActivated, this, 0); - } - }); - } - - // Signal when a socket changes, so we can start the hole punch over. - connect(newNodePointer.data(), &NetworkPeer::socketUpdated, this, [this, weakPtr] { - emit nodeSocketUpdated(weakPtr); - }); - - return newNodePointer; } + + auto removeOldNode = [&](auto node) { + if (node) { + QWriteLocker writeLocker(&_nodeMutex); + _localIDMap.unsafe_erase(node->getLocalID()); + _nodeHash.unsafe_erase(node->getUUID()); + handleNodeKill(node); + } + }; + + // if this is a solo node type, we assume that the DS has replaced its assignment and we should kill the previous node + if (SOLO_NODE_TYPES.count(nodeType)) { + removeOldNode(soloNodeOfType(nodeType)); + } + // If there is a new node with the same socket, this is a reconnection, kill the old node + removeOldNode(findNodeWithAddr(publicSocket)); + removeOldNode(findNodeWithAddr(localSocket)); + + auto it = _connectionIDs.find(uuid); + if (it == _connectionIDs.end()) { + _connectionIDs[uuid] = INITIAL_CONNECTION_ID; + } + + // we didn't have this node, so add them + Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket); + newNode->setIsReplicated(isReplicated); + newNode->setIsUpstream(isUpstream || NodeType::isUpstream(nodeType)); + newNode->setConnectionSecret(connectionSecret); + newNode->setPermissions(permissions); + newNode->setLocalID(localID); + + // move the newly constructed node to the LNL thread + newNode->moveToThread(thread()); + + if (nodeType == NodeType::AudioMixer) { + LimitedNodeList::flagTimeForConnectionStep(LimitedNodeList::AddedAudioMixer); + } + + SharedNodePointer newNodePointer(newNode, &QObject::deleteLater); + + + { + QReadLocker readLocker(&_nodeMutex); + // insert the new node and release our read lock + _nodeHash.insert({ newNode->getUUID(), newNodePointer }); + _localIDMap.insert({ localID, newNodePointer }); + } + + qCDebug(networking) << "Added" << *newNode; + + auto weakPtr = newNodePointer.toWeakRef(); // We don't want the lambdas to hold a strong ref + + emit nodeAdded(newNodePointer); + if (newNodePointer->getActiveSocket()) { + emit nodeActivated(newNodePointer); + } else { + connect(newNodePointer.data(), &NetworkPeer::socketActivated, this, [this, weakPtr] { + auto sharedPtr = weakPtr.lock(); + if (sharedPtr) { + emit nodeActivated(sharedPtr); + disconnect(sharedPtr.data(), &NetworkPeer::socketActivated, this, 0); + } + }); + } + + // Signal when a socket changes, so we can start the hole punch over. + connect(newNodePointer.data(), &NetworkPeer::socketUpdated, this, [this, weakPtr] { + emit nodeSocketUpdated(weakPtr); + }); + + return newNodePointer; } std::unique_ptr LimitedNodeList::constructPingPacket(const QUuid& nodeId, PingType_t pingType) {