From 50965f1916619ee1aca1694d04318503f4bad05d Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 26 Jun 2019 18:04:41 -0700
Subject: [PATCH 1/4] Handle remote public-address changes
---
libraries/networking/src/NetworkPeer.cpp | 24 ++++++++++++------------
libraries/networking/src/NetworkPeer.h | 2 +-
libraries/networking/src/udt/Socket.cpp | 5 +++++
libraries/networking/src/udt/Socket.h | 2 +-
4 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/libraries/networking/src/NetworkPeer.cpp b/libraries/networking/src/NetworkPeer.cpp
index 4e0a82ba0e..a48922726e 100644
--- a/libraries/networking/src/NetworkPeer.cpp
+++ b/libraries/networking/src/NetworkPeer.cpp
@@ -59,13 +59,13 @@ void NetworkPeer::setPublicSocket(const HifiSockAddr& publicSocket) {
bool wasOldSocketNull = _publicSocket.isNull();
- auto temp = _publicSocket.objectName();
+ auto previousSocket = _publicSocket;
_publicSocket = publicSocket;
- _publicSocket.setObjectName(temp);
+ _publicSocket.setObjectName(previousSocket.objectName());
if (!wasOldSocketNull) {
- qCDebug(networking) << "Public socket change for node" << *this;
- emit socketUpdated();
+ qCDebug(networking) << "Public socket change for node" << *this << "; previously" << previousSocket;
+ emit socketUpdated(previousSocket, _publicSocket);
}
}
}
@@ -79,13 +79,13 @@ void NetworkPeer::setLocalSocket(const HifiSockAddr& localSocket) {
bool wasOldSocketNull = _localSocket.isNull();
- auto temp = _localSocket.objectName();
+ auto previousSocket = _localSocket;
_localSocket = localSocket;
- _localSocket.setObjectName(temp);
+ _localSocket.setObjectName(previousSocket.objectName());
if (!wasOldSocketNull) {
- qCDebug(networking) << "Local socket change for node" << *this;
- emit socketUpdated();
+ qCDebug(networking) << "Local socket change for node" << *this << "; previously" << previousSocket;
+ emit socketUpdated(previousSocket, _localSocket);
}
}
}
@@ -99,13 +99,13 @@ void NetworkPeer::setSymmetricSocket(const HifiSockAddr& symmetricSocket) {
bool wasOldSocketNull = _symmetricSocket.isNull();
- auto temp = _symmetricSocket.objectName();
+ auto previousSocket = _symmetricSocket;
_symmetricSocket = symmetricSocket;
- _symmetricSocket.setObjectName(temp);
+ _symmetricSocket.setObjectName(previousSocket.objectName());
if (!wasOldSocketNull) {
- qCDebug(networking) << "Symmetric socket change for node" << *this;
- emit socketUpdated();
+ qCDebug(networking) << "Symmetric socket change for node" << *this << "; previously" << previousSocket;
+ emit socketUpdated(previousSocket, _symmetricSocket);
}
}
}
diff --git a/libraries/networking/src/NetworkPeer.h b/libraries/networking/src/NetworkPeer.h
index e2149d64af..4c08c97d3c 100644
--- a/libraries/networking/src/NetworkPeer.h
+++ b/libraries/networking/src/NetworkPeer.h
@@ -94,7 +94,7 @@ public slots:
signals:
void pingTimerTimeout();
void socketActivated(const HifiSockAddr& sockAddr);
- void socketUpdated();
+ void socketUpdated(HifiSockAddr previousAddress, HifiSockAddr currentAddress);
protected:
void setActiveSocket(HifiSockAddr* discoveredSocket);
diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp
index c56f276560..7cc6680efd 100644
--- a/libraries/networking/src/udt/Socket.cpp
+++ b/libraries/networking/src/udt/Socket.cpp
@@ -538,6 +538,11 @@ void Socket::handleStateChanged(QAbstractSocket::SocketState socketState) {
}
}
+void Socket::handleRemoteAddressChange(HifiSockAddr previousAddress, HifiSockAddr currentAddress) {
+ Lock connectionsLock(_connectionsHashMutex);
+
+}
+
#if (PR_BUILD || DEV_BUILD)
void Socket::sendFakedHandshakeRequest(const HifiSockAddr& sockAddr) {
diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h
index ad9d6de8b8..74eb413bc2 100644
--- a/libraries/networking/src/udt/Socket.h
+++ b/libraries/networking/src/udt/Socket.h
@@ -106,11 +106,11 @@ private slots:
void handleSocketError(QAbstractSocket::SocketError socketError);
void handleStateChanged(QAbstractSocket::SocketState socketState);
+ void handleRemoteAddressChange(HifiSockAddr previousAddress, HifiSockAddr currentAddress);
private:
void setSystemBufferSizes();
Connection* findOrCreateConnection(const HifiSockAddr& sockAddr, bool filterCreation = false);
- bool socketMatchesNodeOrDomain(const HifiSockAddr& sockAddr);
// privatized methods used by UDTTest - they are private since they must be called on the Socket thread
ConnectionStats::Stats sampleStatsForConnection(const HifiSockAddr& destination);
From 1265531f797f535e95ba18cfd696482115050673 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 27 Jun 2019 15:52:39 -0700
Subject: [PATCH 2/4] Plumb down change of destination address
---
libraries/networking/src/udt/Connection.cpp | 8 ++++++++
libraries/networking/src/udt/Connection.h | 2 ++
libraries/networking/src/udt/SendQueue.cpp | 4 ++++
libraries/networking/src/udt/SendQueue.h | 1 +
libraries/networking/src/udt/Socket.cpp | 8 ++++++++
5 files changed, 23 insertions(+)
diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp
index 418dc8f417..872ee4dd4c 100644
--- a/libraries/networking/src/udt/Connection.cpp
+++ b/libraries/networking/src/udt/Connection.cpp
@@ -114,6 +114,7 @@ SendQueue& Connection::getSendQueue() {
QObject::connect(_sendQueue.get(), &SendQueue::packetRetransmitted, this, &Connection::recordRetransmission);
QObject::connect(_sendQueue.get(), &SendQueue::queueInactive, this, &Connection::queueInactive);
QObject::connect(_sendQueue.get(), &SendQueue::timeout, this, &Connection::queueTimeout);
+ QObject::connect(this, &Connection::destinationAddressChange, _sendQueue.get(), &SendQueue::updateDestinationAddress);
// set defaults on the send queue from our congestion control object and estimatedTimeout()
@@ -485,3 +486,10 @@ std::unique_ptr<Packet> PendingReceivedMessage::removeNextPacket() {
}
return std::unique_ptr<Packet>();
}
+
+void Connection::setDestinationAddress(const HifiSockAddr& destination) {
+ if (_destination != destination) {
+ _destination = destination;
+ emit destinationAddressChange(destination);
+ }
+}
diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h
index 938ec36860..47edb021c8 100644
--- a/libraries/networking/src/udt/Connection.h
+++ b/libraries/networking/src/udt/Connection.h
@@ -76,10 +76,12 @@ public:
void recordSentUnreliablePackets(int wireSize, int payloadSize);
void recordReceivedUnreliablePackets(int wireSize, int payloadSize);
+ void setDestinationAddress(const HifiSockAddr& destination);
signals:
void packetSent();
void receiverHandshakeRequestComplete(const HifiSockAddr& sockAddr);
+ void destinationAddressChange(HifiSockAddr currentAddress);
private slots:
void recordSentPackets(int wireSize, int payloadSize, SequenceNumber seqNum, p_high_resolution_clock::time_point timePoint);
diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp
index 15841b5c21..2997c272f9 100644
--- a/libraries/networking/src/udt/SendQueue.cpp
+++ b/libraries/networking/src/udt/SendQueue.cpp
@@ -557,3 +557,7 @@ void SendQueue::deactivate() {
bool SendQueue::isFlowWindowFull() const {
return seqlen(SequenceNumber { (uint32_t) _lastACKSequenceNumber }, _currentSequenceNumber) > _flowWindowSize;
}
+
+void SendQueue::updateDestinationAddress(HifiSockAddr newAddress) {
+ _destination = newAddress;
+}
diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h
index c1a2b59075..2153745250 100644
--- a/libraries/networking/src/udt/SendQueue.h
+++ b/libraries/networking/src/udt/SendQueue.h
@@ -75,6 +75,7 @@ public slots:
void ack(SequenceNumber ack);
void fastRetransmit(SequenceNumber ack);
void handshakeACK();
+ void updateDestinationAddress(HifiSockAddr newAddress);
signals:
void packetSent(int wireSize, int payloadSize, SequenceNumber seqNum, p_high_resolution_clock::time_point timePoint);
diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp
index 7cc6680efd..94628aa852 100644
--- a/libraries/networking/src/udt/Socket.cpp
+++ b/libraries/networking/src/udt/Socket.cpp
@@ -540,7 +540,15 @@ void Socket::handleStateChanged(QAbstractSocket::SocketState socketState) {
void Socket::handleRemoteAddressChange(HifiSockAddr previousAddress, HifiSockAddr currentAddress) {
Lock connectionsLock(_connectionsHashMutex);
+ _connectionsHash.erase(currentAddress);
+ const auto connectionIter = _connectionsHash.find(currentAddress);
+ if (connectionIter != _connectionsHash.end()) {
+ auto connection = std::move(connectionIter->second);
+ _connectionsHash.erase(connectionIter);
+ connection->setDestinationAddress(currentAddress);
+ _connectionsHash[currentAddress] = std::move(connection);
+ }
}
#if (PR_BUILD || DEV_BUILD)
From 9b894456e9a6bdd76c7f6866eb16b791417747ad Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 27 Jun 2019 17:29:17 -0700
Subject: [PATCH 3/4] Change address for sequence-number hash also
---
libraries/networking/src/udt/Socket.cpp | 30 ++++++++++++++++++-------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp
index 94628aa852..e8fe3741aa 100644
--- a/libraries/networking/src/udt/Socket.cpp
+++ b/libraries/networking/src/udt/Socket.cpp
@@ -539,15 +539,29 @@ void Socket::handleStateChanged(QAbstractSocket::SocketState socketState) {
}
void Socket::handleRemoteAddressChange(HifiSockAddr previousAddress, HifiSockAddr currentAddress) {
- Lock connectionsLock(_connectionsHashMutex);
- _connectionsHash.erase(currentAddress);
+ {
+ Lock connectionsLock(_connectionsHashMutex);
+ _connectionsHash.erase(currentAddress);
- const auto connectionIter = _connectionsHash.find(currentAddress);
- if (connectionIter != _connectionsHash.end()) {
- auto connection = std::move(connectionIter->second);
- _connectionsHash.erase(connectionIter);
- connection->setDestinationAddress(currentAddress);
- _connectionsHash[currentAddress] = std::move(connection);
+ const auto connectionIter = _connectionsHash.find(previousAddress);
+ if (connectionIter != _connectionsHash.end()) {
+ auto connection = std::move(connectionIter->second);
+ _connectionsHash.erase(connectionIter);
+ connection->setDestinationAddress(currentAddress);
+ _connectionsHash[currentAddress] = std::move(connection);
+ }
+ }
+
+ {
+ Lock sequenceNumbersLock(_unreliableSequenceNumbersMutex);
+ _unreliableSequenceNumbers.erase(currentAddress);
+
+ const auto sequenceNumbersIter = _unreliableSequenceNumbers.find(previousAddress);
+ if (sequenceNumbersIter != _unreliableSequenceNumbers.end()) {
+ auto sequenceNumbers = sequenceNumbersIter->second;
+ _unreliableSequenceNumbers.erase(sequenceNumbersIter);
+ _unreliableSequenceNumbers[currentAddress] = sequenceNumbers;
+ }
}
}
From 225f65f4fb02df71ce4315973bea8359f8109a9a Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Fri, 28 Jun 2019 11:30:09 -0700
Subject: [PATCH 4/4] Log message tweaks
Also don't need std in std::move, somehow.
---
libraries/networking/src/LimitedNodeList.cpp | 8 +++++---
libraries/networking/src/udt/Socket.cpp | 4 ++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp
index ac0f9e0b07..b5872a46fd 100644
--- a/libraries/networking/src/LimitedNodeList.cpp
+++ b/libraries/networking/src/LimitedNodeList.cpp
@@ -1088,12 +1088,14 @@ void LimitedNodeList::processSTUNResponse(std::unique_ptr<udt::BasePacket> packe
if (parseSTUNResponse(packet.get(), newPublicAddress, newPublicPort)) {
if (newPublicAddress != _publicSockAddr.getAddress() || newPublicPort != _publicSockAddr.getPort()) {
- _publicSockAddr = HifiSockAddr(newPublicAddress, newPublicPort);
-
- qCDebug(networking, "New public socket received from STUN server is %s:%hu",
+ qCDebug(networking, "New public socket received from STUN server is %s:%hu (was %s:%hu)",
+ newPublicAddress.toString().toStdString().c_str(),
+ newPublicPort,
_publicSockAddr.getAddress().toString().toLocal8Bit().constData(),
_publicSockAddr.getPort());
+ _publicSockAddr = HifiSockAddr(newPublicAddress, newPublicPort);
+
if (!_hasCompletedInitialSTUN) {
// if we're here we have definitely completed our initial STUN sequence
stopInitialSTUNUpdate(true);
diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp
index e8fe3741aa..4714160ace 100644
--- a/libraries/networking/src/udt/Socket.cpp
+++ b/libraries/networking/src/udt/Socket.cpp
@@ -545,10 +545,10 @@ void Socket::handleRemoteAddressChange(HifiSockAddr previousAddress, HifiSockAdd
const auto connectionIter = _connectionsHash.find(previousAddress);
if (connectionIter != _connectionsHash.end()) {
- auto connection = std::move(connectionIter->second);
+ auto connection = move(connectionIter->second);
_connectionsHash.erase(connectionIter);
connection->setDestinationAddress(currentAddress);
- _connectionsHash[currentAddress] = std::move(connection);
+ _connectionsHash[currentAddress] = move(connection);
}
}