From 4f11e46b5e9e92d63fcfcd19f54d1353b1115a80 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 24 Sep 2019 16:39:59 -0700 Subject: [PATCH] Don't move Connection class to changed address until it's used --- assignment-client/src/avatars/MixerAvatar.cpp | 6 +++--- libraries/networking/src/udt/Connection.cpp | 8 +++----- libraries/networking/src/udt/Connection.h | 1 + libraries/networking/src/udt/Socket.cpp | 6 ++++-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/assignment-client/src/avatars/MixerAvatar.cpp b/assignment-client/src/avatars/MixerAvatar.cpp index 97d28fa246..ac633d9388 100644 --- a/assignment-client/src/avatars/MixerAvatar.cpp +++ b/assignment-client/src/avatars/MixerAvatar.cpp @@ -38,7 +38,7 @@ MixerAvatar::MixerAvatar() { _pendingEvent = false; _verifyState = verificationFailed; _needsIdentityUpdate = true; - qCDebug(avatars) << "Dynamic verification TIMED-OUT for " << getDisplayName() << getSessionUUID(); + qCDebug(avatars) << "Dynamic verification TIMED-OUT for" << getDisplayName() << getSessionUUID(); } else { qCDebug(avatars) << "Ignoring timeout of avatar challenge"; } @@ -287,7 +287,7 @@ void MixerAvatar::processCertifyEvents() { << ":" << _dynamicMarketResponse; } } else { - qCDebug(avatars) << "Get owner status failed for " << getDisplayName() << _marketplaceIdFromURL << + qCDebug(avatars) << "Get owner status failed for" << getDisplayName() << _marketplaceIdFromURL << "message:" << responseJson["message"].toString(); _verifyState = error; } @@ -356,7 +356,7 @@ void MixerAvatar::processChallengeResponse(ReceivedMessage& response) { _verifyState = challengeResult ? verificationSucceeded : verificationFailed; _needsIdentityUpdate = true; if (_verifyState == verificationFailed) { - qCDebug(avatars) << "Dynamic verification FAILED for " << getDisplayName() << getSessionUUID(); + qCDebug(avatars) << "Dynamic verification FAILED for" << getDisplayName() << getSessionUUID(); } else { qCDebug(avatars) << "Dynamic verification SUCCEEDED for" << getDisplayName() << getSessionUUID(); } diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 872ee4dd4c..338b95163f 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -269,7 +269,7 @@ bool Connection::processReceivedSequenceNumber(SequenceNumber sequenceNumber, in bool wasDuplicate = false; if (sequenceNumber > _lastReceivedSequenceNumber) { - // Update largest recieved sequence number + // Update largest received sequence number _lastReceivedSequenceNumber = sequenceNumber; } else { // Otherwise, it could be a resend, try and remove it from the loss list @@ -312,9 +312,7 @@ void Connection::processControl(ControlPacketPointer controlPacket) { // We're already in a state where we've received a handshake ack, so we are likely in a state // where the other end expired our connection. Let's reset. -#ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "Got HandshakeRequest from" << _destination << ", stopping SendQueue"; -#endif + qCDebug(networking) << "Got HandshakeRequest from" << _destination << "while active, stopping SendQueue"; _hasReceivedHandshakeACK = false; stopSendQueue(); } @@ -333,7 +331,7 @@ void Connection::processACK(ControlPacketPointer controlPacket) { // validate that this isn't a BS ACK if (ack > getSendQueue().getCurrentSequenceNumber()) { // in UDT they specifically break the connection here - do we want to do anything? - Q_ASSERT_X(false, "Connection::processACK", "ACK recieved higher than largest sent sequence number"); + Q_ASSERT_X(false, "Connection::processACK", "ACK received higher than largest sent sequence number"); return; } diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 47edb021c8..00d5beb5ab 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -73,6 +73,7 @@ public: void setMaxBandwidth(int maxBandwidth); void sendHandshakeRequest(); + bool hasReceivedHandshake() const { return _hasReceivedHandshake; } void recordSentUnreliablePackets(int wireSize, int payloadSize); void recordReceivedUnreliablePackets(int wireSize, int payloadSize); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 20cb30dbd8..5b3e2e1b5b 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -261,7 +261,7 @@ Connection* Socket::findOrCreateConnection(const HifiSockAddr& sockAddr, bool fi // we did not have a matching connection, time to see if we should make one if (filterCreate && _connectionCreationFilterOperator && !_connectionCreationFilterOperator(sockAddr)) { - // the connection creation filter did not allow us to create a new connection + // the connection creation filter did not allow us to create a new connectionclientHandshakeRequestComplete #ifdef UDT_CONNECTION_DEBUG qCDebug(networking) << "Socket::findOrCreateConnection refusing to create Connection class for" << sockAddr << "due to connection creation filter"; @@ -548,12 +548,14 @@ void Socket::handleRemoteAddressChange(HifiSockAddr previousAddress, HifiSockAdd Lock connectionsLock(_connectionsHashMutex); const auto connectionIter = _connectionsHash.find(previousAddress); - if (connectionIter != _connectionsHash.end()) { + // Don't move classes that are unused so far. + if (connectionIter != _connectionsHash.end() && connectionIter->second->hasReceivedHandshake()) { auto connection = move(connectionIter->second); _connectionsHash.erase(connectionIter); connection->setDestinationAddress(currentAddress); _connectionsHash[currentAddress] = move(connection); connectionsLock.unlock(); + qCDebug(networking) << "Moved Connection class from" << previousAddress << "to" << currentAddress; Lock sequenceNumbersLock(_unreliableSequenceNumbersMutex); const auto sequenceNumbersIter = _unreliableSequenceNumbers.find(previousAddress);