From 4f11e46b5e9e92d63fcfcd19f54d1353b1115a80 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 24 Sep 2019 16:39:59 -0700 Subject: [PATCH 1/3] 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); From 5067729c5215aa9004523356f98ac2d6e8891286 Mon Sep 17 00:00:00 2001 From: Clement Date: Tue, 24 Sep 2019 16:25:44 -0700 Subject: [PATCH 2/3] Add flag for account settings feature --- interface/src/Application.cpp | 4 ++-- libraries/networking/src/AccountManager.cpp | 13 +++++++++++-- libraries/networking/src/AccountManager.h | 3 ++- tools/ac-client/src/ACClientApp.cpp | 2 +- tools/atp-client/src/ATPClientApp.cpp | 2 +- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ef3aafef4f..6a320e53ee 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -856,9 +856,9 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) { DependencyManager::set(); DependencyManager::set(); #if defined(Q_OS_ANDROID) - DependencyManager::set(); // use the default user agent getter + DependencyManager::set(true); // use the default user agent getter #else - DependencyManager::set(std::bind(&Application::getUserAgent, qApp)); + DependencyManager::set(true, std::bind(&Application::getUserAgent, qApp)); #endif DependencyManager::set(); DependencyManager::set(ScriptEngine::CLIENT_SCRIPT, defaultScriptsOverrideOption); diff --git a/libraries/networking/src/AccountManager.cpp b/libraries/networking/src/AccountManager.cpp index a31d117f59..3de39902b1 100644 --- a/libraries/networking/src/AccountManager.cpp +++ b/libraries/networking/src/AccountManager.cpp @@ -73,9 +73,10 @@ QJsonObject AccountManager::dataObjectFromResponse(QNetworkReply* requestReply) } } -AccountManager::AccountManager(UserAgentGetter userAgentGetter) : +AccountManager::AccountManager(bool accountSettingsEnabled, UserAgentGetter userAgentGetter) : _userAgentGetter(userAgentGetter), - _authURL() + _authURL(), + _accountSettingsEnabled(accountSettingsEnabled) { qRegisterMetaType("OAuthAccessToken"); qRegisterMetaTypeStreamOperators("OAuthAccessToken"); @@ -796,6 +797,10 @@ void AccountManager::requestProfileError(QNetworkReply::NetworkError error) { } void AccountManager::requestAccountSettings() { + if (!_accountSettingsEnabled) { + return; + } + QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance(); QUrl lockerURL = _authURL; @@ -840,6 +845,10 @@ void AccountManager::requestAccountSettingsError(QNetworkReply::NetworkError err } void AccountManager::postAccountSettings() { + if (!_accountSettingsEnabled) { + return; + } + if (_settings.lastChangeTimestamp() <= _lastSuccessfulSyncTimestamp && _lastSuccessfulSyncTimestamp != 0) { // Nothing changed, skipping settings post return; diff --git a/libraries/networking/src/AccountManager.h b/libraries/networking/src/AccountManager.h index f29221d671..8c7789c218 100644 --- a/libraries/networking/src/AccountManager.h +++ b/libraries/networking/src/AccountManager.h @@ -60,7 +60,7 @@ const auto DEFAULT_USER_AGENT_GETTER = []() -> QString { return HIGH_FIDELITY_US class AccountManager : public QObject, public Dependency { Q_OBJECT public: - AccountManager(UserAgentGetter userAgentGetter = DEFAULT_USER_AGENT_GETTER); + AccountManager(bool accountSettingsEnabled = false, UserAgentGetter userAgentGetter = DEFAULT_USER_AGENT_GETTER); QNetworkRequest createRequest(QString path, AccountManagerAuth::Type authType); Q_INVOKABLE void sendRequest(const QString& path, @@ -182,6 +182,7 @@ private: bool _limitedCommerce { false }; QString _configFileURL; + bool _accountSettingsEnabled { false }; AccountSettings _settings; quint64 _currentSyncTimestamp { 0 }; quint64 _lastSuccessfulSyncTimestamp { 0 }; diff --git a/tools/ac-client/src/ACClientApp.cpp b/tools/ac-client/src/ACClientApp.cpp index 6b781a5bea..24805a3348 100644 --- a/tools/ac-client/src/ACClientApp.cpp +++ b/tools/ac-client/src/ACClientApp.cpp @@ -100,7 +100,7 @@ ACClientApp::ACClientApp(int argc, char* argv[]) : DependencyManager::registerInheritance(); - DependencyManager::set([&]{ return QString("Mozilla/5.0 (HighFidelityACClient)"); }); + DependencyManager::set(false, [&]{ return QString("Mozilla/5.0 (HighFidelityACClient)"); }); DependencyManager::set(); DependencyManager::set(NodeType::Agent, listenPort); diff --git a/tools/atp-client/src/ATPClientApp.cpp b/tools/atp-client/src/ATPClientApp.cpp index 8c2fd44adf..b2e7db5c87 100644 --- a/tools/atp-client/src/ATPClientApp.cpp +++ b/tools/atp-client/src/ATPClientApp.cpp @@ -138,7 +138,7 @@ ATPClientApp::ATPClientApp(int argc, char* argv[]) : DependencyManager::registerInheritance(); DependencyManager::set(); - DependencyManager::set([&]{ return QString(HIGH_FIDELITY_ATP_CLIENT_USER_AGENT); }); + DependencyManager::set(false, [&]{ return QString(HIGH_FIDELITY_ATP_CLIENT_USER_AGENT); }); DependencyManager::set(); DependencyManager::set(NodeType::Agent, _listenPort); From 6d7f71199019ec5fb077e08f28bfcea2ca7ea944 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 25 Sep 2019 09:33:49 -0700 Subject: [PATCH 3/3] Remove typo that somehow got into a comment --- libraries/networking/src/udt/Socket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 5b3e2e1b5b..7e1ab02a76 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 connectionclientHandshakeRequestComplete + // the connection creation filter did not allow us to create a new connection #ifdef UDT_CONNECTION_DEBUG qCDebug(networking) << "Socket::findOrCreateConnection refusing to create Connection class for" << sockAddr << "due to connection creation filter";