From 4039c2e3e0fc5995b10a431c18f01a02eee307d5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 26 Aug 2015 14:27:12 -0700 Subject: [PATCH] fix for asset-server naming, deadlock in timeout/wait --- domain-server/src/DomainGatekeeper.cpp | 13 +++++++++++- libraries/networking/src/LimitedNodeList.cpp | 3 +++ libraries/networking/src/udt/SendQueue.cpp | 21 ++++++++++++++++++-- libraries/networking/src/udt/SendQueue.h | 2 +- libraries/networking/src/udt/Socket.cpp | 11 ++++++++++ libraries/networking/src/udt/Socket.h | 1 + 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp index 72b353f8a0..07afa007c5 100644 --- a/domain-server/src/DomainGatekeeper.cpp +++ b/domain-server/src/DomainGatekeeper.cpp @@ -48,7 +48,8 @@ QUuid DomainGatekeeper::assignmentUUIDForPendingAssignment(const QUuid& tempUUID } const NodeSet STATICALLY_ASSIGNED_NODES = NodeSet() << NodeType::AudioMixer - << NodeType::AvatarMixer << NodeType::EntityServer; + << NodeType::AvatarMixer << NodeType::EntityServer + << NodeType::AssetServer; void DomainGatekeeper::processConnectRequestPacket(QSharedPointer packet) { if (packet->getPayloadSize() == 0) { @@ -65,6 +66,16 @@ void DomainGatekeeper::processConnectRequestPacket(QSharedPointer pack return; } + static const NodeSet VALID_NODE_TYPES { + NodeType::AudioMixer, NodeType::AvatarMixer, NodeType::AssetServer, NodeType::EntityServer, NodeType::Agent + }; + + if (!VALID_NODE_TYPES.contains(nodeConnection.nodeType)) { + qDebug() << "Received an invalid node type with connect request. Will not allow connection from" + << nodeConnection.senderSockAddr; + return; + } + // check if this connect request matches an assignment in the queue auto pendingAssignment = _pendingAssignedNodes.find(nodeConnection.connectUUID); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index c342f660a3..e5923b059f 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -409,6 +409,9 @@ void LimitedNodeList::eraseAllNodes() { void LimitedNodeList::reset() { eraseAllNodes(); + + // we need to make sure any socket connections are gone so wait on that here + _nodeSocket.clearConnections(); } void LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID) { diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index da702c7166..c0638a60f7 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -134,6 +134,12 @@ void SendQueue::queuePacketList(std::unique_ptr packetList) { void SendQueue::stop() { _isRunning = false; + + // in case we're waiting to send another handshake, release the condition_variable now so we cleanup sooner + _handshakeACKCondition.notify_one(); + + // in case the empty condition is waiting for packets/loss release it now so that the queue is cleaned up + _emptyCondition.notify_one(); } void SendQueue::sendPacket(const Packet& packet) { @@ -286,14 +292,22 @@ void SendQueue::run() { } _flowWindowWasFull = flowWindowFull; + // since we're a while loop, give the thread a chance to process events + QCoreApplication::processEvents(); + + // we just processed events so check now if we were just told to stop + if (!_isRunning) { + break; + } if (_hasReceivedHandshakeACK && !sentAPacket) { static const std::chrono::seconds CONSIDER_INACTIVE_AFTER { 5 }; if (flowWindowFull && (high_resolution_clock::now() - _flowWindowFullSince) > CONSIDER_INACTIVE_AFTER) { // If the flow window has been full for over CONSIDER_INACTIVE_AFTER, - // then signal the queue is inactive + // then signal the queue is inactive and return so it can be cleaned up emit queueInactive(); + return; } else { // During our processing above we didn't send any packets and the flow window is not full. @@ -303,15 +317,18 @@ void SendQueue::run() { // The packets queue and loss list mutexes are now both locked - check if they're still both empty if (doubleLock.try_lock() && _packets.empty() && _naks.getLength() == 0) { + // both are empty - let's use a condition_variable_any to wait auto cvStatus = _emptyCondition.wait_for(doubleLock, CONSIDER_INACTIVE_AFTER); // we have the double lock again - Make sure to unlock it doubleLock.unlock(); - // Check if we've been inactive for too long if (cvStatus == std::cv_status::timeout) { + // the wait_for released because we've been inactive for too long + // so emit our inactive signal and return so the send queue can be cleaned up emit queueInactive(); + return; } // skip to the next iteration diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 89dc2fae8f..70493f2054 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -83,7 +83,7 @@ private: void sendNewPacketAndAddToSentList(std::unique_ptr newPacket, SequenceNumber sequenceNumber); bool maybeSendNewPacket(); // Figures out what packet to send next - bool maybeResendPacket(); // Determines whether to resend a packet and wich one + bool maybeResendPacket(); // Determines whether to resend a packet and which one // Increments current sequence number and return it SequenceNumber getNextSequenceNumber(); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index d5e57df60b..db0377de05 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -154,6 +154,17 @@ Connection& Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { return *it->second; } +void Socket::clearConnections() { + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, "clearConnections", Qt::BlockingQueuedConnection); + return; + } + + // clear all of the current connections in the socket + qDebug() << "Clearing all remaining connections in Socket."; + _connectionsHash.clear(); +} + void Socket::cleanupConnection(HifiSockAddr sockAddr) { qCDebug(networking) << "Socket::cleanupConnection called for connection to" << sockAddr; _connectionsHash.erase(sockAddr); diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 0014a97e2b..f421b98288 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -75,6 +75,7 @@ public: public slots: void cleanupConnection(HifiSockAddr sockAddr); + void clearConnections(); private slots: void readPendingDatagrams();