From c332244f4f7ebe525f0d7ce973c28805f413d45d Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 15 Jun 2017 17:34:20 -0700 Subject: [PATCH] fix race for isUpstream and node hole punch --- assignment-client/src/audio/AudioMixer.cpp | 3 +-- assignment-client/src/avatars/AvatarMixer.cpp | 3 +-- libraries/networking/src/LimitedNodeList.cpp | 12 ++++++++---- libraries/networking/src/LimitedNodeList.h | 5 +++-- libraries/networking/src/Node.cpp | 8 ++------ libraries/networking/src/Node.h | 1 - libraries/networking/src/NodeList.cpp | 2 +- 7 files changed, 16 insertions(+), 18 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 76ce2061c0..d0afc7b0d4 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -126,9 +126,8 @@ void AudioMixer::queueReplicatedAudioPacket(QSharedPointer mess auto replicatedNode = nodeList->addOrUpdateNode(nodeID, NodeType::Agent, message->getSenderSockAddr(), message->getSenderSockAddr(), - DEFAULT_AGENT_PERMISSIONS, true); + true, true); replicatedNode->setLastHeardMicrostamp(usecTimestampNow()); - replicatedNode->setIsUpstream(true); // construct a "fake" audio received message from the byte array and packet list information auto audioData = message->getMessage().mid(NUM_BYTES_RFC4122_UUID); diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 4998a3b520..c4e59b6324 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -75,10 +75,9 @@ SharedNodePointer addOrUpdateReplicatedNode(const QUuid& nodeID, const HifiSockA auto replicatedNode = DependencyManager::get()->addOrUpdateNode(nodeID, NodeType::Agent, senderSockAddr, senderSockAddr, - DEFAULT_AGENT_PERMISSIONS, true); + true, true); replicatedNode->setLastHeardMicrostamp(usecTimestampNow()); - replicatedNode->setIsUpstream(true); return replicatedNode; } diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 14a9d0b257..d2bae28820 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -570,9 +570,8 @@ void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) { SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, - const NodePermissions& permissions, - bool isReplicated, - const QUuid& connectionSecret) { + bool isReplicated, bool isUpstream, + const QUuid& connectionSecret, const NodePermissions& permissions) { QReadLocker readLocker(&_nodeMutex); NodeHash::const_iterator it = _nodeHash.find(uuid); @@ -584,11 +583,16 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t matchingNode->setPermissions(permissions); matchingNode->setConnectionSecret(connectionSecret); matchingNode->setIsReplicated(isReplicated); + matchingNode->setIsUpstream(isUpstream); return matchingNode; } else { // we didn't have this node, so add them - Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket, permissions, isReplicated, connectionSecret); + Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket); + newNode->setIsReplicated(isReplicated); + newNode->setIsUpstream(isUpstream); + newNode->setConnectionSecret(connectionSecret); + newNode->setPermissions(permissions); // move the newly constructed node to the LNL thread newNode->moveToThread(thread()); diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 5e2c88ed94..01e0ef2dc0 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -145,8 +145,9 @@ public: SharedNodePointer addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, - const NodePermissions& permissions = DEFAULT_AGENT_PERMISSIONS, - bool isReplicated = false, const QUuid& connectionSecret = QUuid()); + bool isReplicated = false, bool isUpstream = false, + const QUuid& connectionSecret = QUuid(), + const NodePermissions& permissions = DEFAULT_AGENT_PERMISSIONS); static bool parseSTUNResponse(udt::BasePacket* packet, QHostAddress& newPublicAddress, uint16_t& newPublicPort); bool hasCompletedInitialSTUN() const { return _hasCompletedInitialSTUN; } diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index f74be8adcd..ea1b6e0eb5 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -73,17 +73,13 @@ NodeType_t NodeType::fromString(QString type) { Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket, - const HifiSockAddr& localSocket, const NodePermissions& permissions, bool isReplicated, - const QUuid& connectionSecret, QObject* parent) : + const HifiSockAddr& localSocket, QObject* parent) : NetworkPeer(uuid, publicSocket, localSocket, parent), _type(type), - _connectionSecret(connectionSecret), - _isReplicated(isReplicated), _pingMs(-1), // "Uninitialized" _clockSkewUsec(0), _mutex(), - _clockSkewMovingPercentile(30, 0.8f), // moving 80th percentile of 30 samples - _permissions(permissions) + _clockSkewMovingPercentile(30, 0.8f) // moving 80th percentile of 30 samples { // Update socket's object name setType(_type); diff --git a/libraries/networking/src/Node.h b/libraries/networking/src/Node.h index 33c9e2c205..c20ff5a395 100644 --- a/libraries/networking/src/Node.h +++ b/libraries/networking/src/Node.h @@ -40,7 +40,6 @@ public: Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, - const NodePermissions& permissions, bool isReplicated, const QUuid& connectionSecret = QUuid(), QObject* parent = nullptr); bool operator==(const Node& otherNode) const { return _uuid == otherNode._uuid; } diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 179b6e202d..f502cb2a3c 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -667,7 +667,7 @@ void NodeList::parseNodeFromPacketStream(QDataStream& packetStream) { packetStream >> connectionUUID; SharedNodePointer node = addOrUpdateNode(nodeUUID, nodeType, nodePublicSocket, - nodeLocalSocket, permissions, isReplicated, connectionUUID); + nodeLocalSocket, isReplicated, false, connectionUUID, permissions); // nodes that are downstream of our own type are kept alive when we hear about them from the domain server if (node->getType() == NodeType::downstreamType(_ownerType)) {