Merge pull request #6425 from birarda/handshake-ds-settings

fix case where servers could run without settings

assignment-client/src/audio/AudioMixer.cpp

@@ -645,8 +645,17 @@ void AudioMixer::sendStatsPacket() {
 
 void AudioMixer::run() {
     
-    ThreadedAssignment::commonInit(AUDIO_MIXER_LOGGING_TARGET_NAME, NodeType::AudioMixer);
+    qDebug() << "Waiting for connection to domain to request settings from domain-server.";
     
+    // wait until we have the domain-server settings, otherwise we bail
+    DomainHandler& domainHandler = DependencyManager::get<NodeList>()->getDomainHandler();
+    connect(&domainHandler, &DomainHandler::settingsReceived, this, &AudioMixer::domainSettingsRequestComplete);
+    connect(&domainHandler, &DomainHandler::settingsReceiveFail, this, &AudioMixer::domainSettingsRequestFailed);
+    
+    ThreadedAssignment::commonInit(AUDIO_MIXER_LOGGING_TARGET_NAME, NodeType::AudioMixer);
+}
+
+void AudioMixer::domainSettingsRequestComplete() {
     auto nodeList = DependencyManager::get<NodeList>();
     
     nodeList->addNodeTypeToInterestSet(NodeType::Agent);
@@ -655,29 +664,19 @@ void AudioMixer::run() {
         node->setLinkedData(new AudioMixerClientData());
     };
     
-    // wait until we have the domain-server settings, otherwise we bail
     DomainHandler& domainHandler = nodeList->getDomainHandler();
-
-    qDebug() << "Waiting for domain settings from domain-server.";
-
-    // block until we get the settingsRequestComplete signal
-    QEventLoop loop;
-    connect(&domainHandler, &DomainHandler::settingsReceived, &loop, &QEventLoop::quit);
-    connect(&domainHandler, &DomainHandler::settingsReceiveFail, &loop, &QEventLoop::quit);
-    domainHandler.requestDomainSettings();
-    loop.exec();
-    
-    if (domainHandler.getSettingsObject().isEmpty()) {
-        qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment.";
-        setFinished(true);
-        return;
-    }
-
     const QJsonObject& settingsObject = domainHandler.getSettingsObject();
     
     // check the settings object to see if we have anything we can parse out
     parseSettingsObject(settingsObject);
     
+    // queue up a connection to start broadcasting mixes now that we're ready to go
+    QMetaObject::invokeMethod(this, "broadcastMixes", Qt::QueuedConnection);
+}
+
+void AudioMixer::broadcastMixes() {
+    auto nodeList = DependencyManager::get<NodeList>(); 
+    
     int nextFrame = 0;
     QElapsedTimer timer;
     timer.start();

assignment-client/src/audio/AudioMixer.h

@@ -40,10 +40,13 @@ public slots:
     static const InboundAudioStream::Settings& getStreamSettings() { return _streamSettings; }
 
 private slots:
+    void broadcastMixes();
     void handleNodeAudioPacket(QSharedPointer<NLPacket> packet, SharedNodePointer sendingNode);
     void handleMuteEnvironmentPacket(QSharedPointer<NLPacket> packet, SharedNodePointer sendingNode);
 
 private:    
+    void domainSettingsRequestComplete();
+    
     /// adds one stream to the mix for a listening node
     int addStreamToMixForListeningNodeWithStream(AudioMixerClientData* listenerNodeData,
                                                     const QUuid& streamUUID,

assignment-client/src/avatars/AvatarMixer.cpp

@@ -71,7 +71,6 @@ const float BILLBOARD_AND_IDENTITY_SEND_PROBABILITY = 1.0f / 187.0f;
 //    1) use the view frustum to cull those avatars that are out of view. Since avatar data doesn't need to be present
 //       if the avatar is not in view or in the keyhole.
 void AvatarMixer::broadcastAvatarData() {
-
     int idleTime = QDateTime::currentMSecsSinceEpoch() - _lastFrameTimestamp;
 
     ++_numStatFrames;
@@ -513,15 +512,15 @@ void AvatarMixer::sendStatsPacket() {
 }
 
 void AvatarMixer::run() {
+    qDebug() << "Waiting for connection to domain to request settings from domain-server.";
+    
+    // wait until we have the domain-server settings, otherwise we bail
+    DomainHandler& domainHandler = DependencyManager::get<NodeList>()->getDomainHandler();
+    connect(&domainHandler, &DomainHandler::settingsReceived, this, &AvatarMixer::domainSettingsRequestComplete);
+    connect(&domainHandler, &DomainHandler::settingsReceiveFail, this, &AvatarMixer::domainSettingsRequestFailed);
+    
     ThreadedAssignment::commonInit(AVATAR_MIXER_LOGGING_NAME, NodeType::AvatarMixer);
 
-    auto nodeList = DependencyManager::get<NodeList>();
-    nodeList->addNodeTypeToInterestSet(NodeType::Agent);
-
-    nodeList->linkedDataCreateCallback = [] (Node* node) {
-        node->setLinkedData(new AvatarMixerClientData());
-    };
-
     // setup the timer that will be fired on the broadcast thread
     _broadcastTimer = new QTimer;
     _broadcastTimer->setInterval(AVATAR_DATA_SEND_INTERVAL_MSECS);
@@ -530,33 +529,24 @@ void AvatarMixer::run() {
     // connect appropriate signals and slots
     connect(_broadcastTimer, &QTimer::timeout, this, &AvatarMixer::broadcastAvatarData, Qt::DirectConnection);
     connect(&_broadcastThread, SIGNAL(started()), _broadcastTimer, SLOT(start()));
+}
 
-    // wait until we have the domain-server settings, otherwise we bail
-    DomainHandler& domainHandler = nodeList->getDomainHandler();
+void AvatarMixer::domainSettingsRequestComplete() {
+    auto nodeList = DependencyManager::get<NodeList>();
+    nodeList->addNodeTypeToInterestSet(NodeType::Agent);
     
-    qDebug() << "Waiting for domain settings from domain-server.";
-
-    // block until we get the settingsRequestComplete signal
-
-    QEventLoop loop;
-    connect(&domainHandler, &DomainHandler::settingsReceived, &loop, &QEventLoop::quit);
-    connect(&domainHandler, &DomainHandler::settingsReceiveFail, &loop, &QEventLoop::quit);
-    domainHandler.requestDomainSettings();
-    loop.exec();
-
-    if (domainHandler.getSettingsObject().isEmpty()) {
-        qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment.";
-        setFinished(true);
-        return;
-    }
+    nodeList->linkedDataCreateCallback = [] (Node* node) {
+        node->setLinkedData(new AvatarMixerClientData());
+    };
     
     // parse the settings to pull out the values we need
-    parseDomainServerSettings(domainHandler.getSettingsObject());
+    parseDomainServerSettings(nodeList->getDomainHandler().getSettingsObject());
     
     // start the broadcastThread
     _broadcastThread.start();
 }
 
+
 void AvatarMixer::parseDomainServerSettings(const QJsonObject& domainSettings) {
     const QString AVATAR_MIXER_SETTINGS_KEY = "avatar_mixer";
     const QString NODE_SEND_BANDWIDTH_KEY = "max_node_send_bandwidth";

assignment-client/src/avatars/AvatarMixer.h

@@ -36,6 +36,7 @@ private slots:
     void handleAvatarIdentityPacket(QSharedPointer<NLPacket> packet, SharedNodePointer senderNode);
     void handleAvatarBillboardPacket(QSharedPointer<NLPacket> packet, SharedNodePointer senderNode);
     void handleKillAvatarPacket(QSharedPointer<NLPacket> packet);
+    void domainSettingsRequestComplete();
     
 private:
     void broadcastAvatarData();

assignment-client/src/entities/EntityServer.cpp

@@ -253,7 +253,7 @@ void EntityServer::pruneDeletedEntities() {
     }
 }
 
-bool EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectionObject) {
+void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectionObject) {
     bool wantEditLogging = false;
     readOptionBool(QString("wantEditLogging"), settingsSectionObject, wantEditLogging);
     qDebug("wantEditLogging=%s", debug::valueOf(wantEditLogging));
@@ -265,6 +265,4 @@ bool EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio
     EntityTreePointer tree = std::static_pointer_cast<EntityTree>(_tree);
     tree->setWantEditLogging(wantEditLogging);
     tree->setWantTerseEditLogging(wantTerseEditLogging);
-
-    return true;
 }

assignment-client/src/entities/EntityServer.h

@@ -41,7 +41,7 @@ public:
     virtual int sendSpecialPackets(const SharedNodePointer& node, OctreeQueryNode* queryNode, int& packetsSent) override;
 
     virtual void entityCreated(const EntityItem& newEntity, const SharedNodePointer& senderNode) override;
-    virtual bool readAdditionalConfiguration(const QJsonObject& settingsSectionObject) override;
+    virtual void readAdditionalConfiguration(const QJsonObject& settingsSectionObject) override;
 
 public slots:
     void pruneDeletedEntities();

assignment-client/src/octree/OctreeServer.cpp

@@ -951,31 +951,14 @@ bool OctreeServer::readOptionString(const QString& optionName, const QJsonObject
     return optionAvailable;
 }
 
-bool OctreeServer::readConfiguration() {
+void OctreeServer::readConfiguration() {
     // if the assignment had a payload, read and parse that
     if (getPayload().size() > 0) {
         parsePayload();
     }
     
-    // wait until we have the domain-server settings, otherwise we bail
-    auto nodeList = DependencyManager::get<NodeList>();
-    DomainHandler& domainHandler = nodeList->getDomainHandler();
+    const QJsonObject& settingsObject = DependencyManager::get<NodeList>()->getDomainHandler().getSettingsObject();
 
-    qDebug() << "Waiting for domain settings from domain-server.";
-
-    // block until we get the settingsRequestComplete signal
-    QEventLoop loop;
-    connect(&domainHandler, &DomainHandler::settingsReceived, &loop, &QEventLoop::quit);
-    connect(&domainHandler, &DomainHandler::settingsReceiveFail, &loop, &QEventLoop::quit);
-    domainHandler.requestDomainSettings();
-    loop.exec();
-
-    if (domainHandler.getSettingsObject().isEmpty()) {
-        qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment.";
-        return false;
-    }
-
-    const QJsonObject& settingsObject = domainHandler.getSettingsObject();
     QString settingsKey = getMyDomainSettingsKey();
     QJsonObject settingsSectionObject = settingsObject[settingsKey].toObject();
     _settings = settingsSectionObject; // keep this for later
@@ -1084,16 +1067,10 @@ bool OctreeServer::readConfiguration() {
                     packetsPerSecondTotalMax, _packetsTotalPerInterval);
 
 
-    return readAdditionalConfiguration(settingsSectionObject);
+    readAdditionalConfiguration(settingsSectionObject);
 }
 
 void OctreeServer::run() {
-
-    auto& packetReceiver = DependencyManager::get<NodeList>()->getPacketReceiver();
-    packetReceiver.registerListener(getMyQueryMessageType(), this, "handleOctreeQueryPacket");
-    packetReceiver.registerListener(PacketType::OctreeDataNack, this, "handleOctreeDataNackPacket");
-    packetReceiver.registerListener(PacketType::JurisdictionRequest, this, "handleJurisdictionRequestPacket");
-    
     _safeServerName = getMyServerName();
 
     // Before we do anything else, create our tree...
@@ -1101,22 +1078,30 @@ void OctreeServer::run() {
     _tree = createTree();
     _tree->setIsServer(true);
     
-    // make sure our NodeList knows what type we are
-    auto nodeList = DependencyManager::get<NodeList>();
-    nodeList->setOwnerType(getMyNodeType());
+    qDebug() << "Waiting for connection to domain to request settings from domain-server.";
+    
+    // wait until we have the domain-server settings, otherwise we bail
+    DomainHandler& domainHandler = DependencyManager::get<NodeList>()->getDomainHandler();
+    connect(&domainHandler, &DomainHandler::settingsReceived, this, &OctreeServer::domainSettingsRequestComplete);
+    connect(&domainHandler, &DomainHandler::settingsReceiveFail, this, &OctreeServer::domainSettingsRequestFailed);
 
     // use common init to setup common timers and logging
     commonInit(getMyLoggingServerTargetName(), getMyNodeType());
+}
+
+void OctreeServer::domainSettingsRequestComplete() {
+    
+    auto nodeList = DependencyManager::get<NodeList>();
     
     // we need to ask the DS about agents so we can ping/reply with them
     nodeList->addNodeTypeToInterestSet(NodeType::Agent);
     
-    // read the configuration from either the payload or the domain server configuration
-    if (!readConfiguration()) {
-        qDebug() << "OctreeServer bailing on run since readConfiguration has failed.";
-        setFinished(true);
-        return; // bailing on run, because readConfiguration failed
-    }
+    auto& packetReceiver = DependencyManager::get<NodeList>()->getPacketReceiver();
+    packetReceiver.registerListener(getMyQueryMessageType(), this, "handleOctreeQueryPacket");
+    packetReceiver.registerListener(PacketType::OctreeDataNack, this, "handleOctreeDataNackPacket");
+    packetReceiver.registerListener(PacketType::JurisdictionRequest, this, "handleJurisdictionRequestPacket");
+    
+    readConfiguration();
     
     beforeRun(); // after payload has been processed
     
@@ -1144,8 +1129,6 @@ void OctreeServer::run() {
         _persistThread->initialize(true);
     }
     
-    HifiSockAddr senderSockAddr;
-
     // set up our jurisdiction broadcaster...
     if (_jurisdiction) {
         _jurisdiction->setNodeType(getMyNodeType());
@@ -1168,6 +1151,7 @@ void OctreeServer::run() {
     if (gmtm) {
         strftime(utcBuffer, MAX_TIME_LENGTH, " [%m/%d/%Y %X UTC]", gmtm);
     }
+    
     qDebug() << "Now running... started at: " << localBuffer << utcBuffer;
 }
 

assignment-client/src/octree/OctreeServer.h

@@ -129,6 +129,7 @@ public slots:
     void sendStatsPacket();
 
 private slots:
+    void domainSettingsRequestComplete();
     void handleOctreeQueryPacket(QSharedPointer<NLPacket> packet, SharedNodePointer senderNode);
     void handleOctreeDataNackPacket(QSharedPointer<NLPacket> packet, SharedNodePointer senderNode);
     void handleJurisdictionRequestPacket(QSharedPointer<NLPacket> packet, SharedNodePointer senderNode);
@@ -138,8 +139,8 @@ protected:
     bool readOptionBool(const QString& optionName, const QJsonObject& settingsSectionObject, bool& result);
     bool readOptionInt(const QString& optionName, const QJsonObject& settingsSectionObject, int& result);
     bool readOptionString(const QString& optionName, const QJsonObject& settingsSectionObject, QString& result);
-    bool readConfiguration();
-    virtual bool readAdditionalConfiguration(const QJsonObject& settingsSectionObject) { return true;  };
+    void readConfiguration();
+    virtual void readAdditionalConfiguration(const QJsonObject& settingsSectionObject) { };
     void parsePayload();
     void initHTTPManager(int port);
     void resetSendingStats();

libraries/networking/src/DomainHandler.cpp

@@ -38,12 +38,17 @@ DomainHandler::DomainHandler(QObject* parent) :
     _icePeer(this),
     _isConnected(false),
     _settingsObject(),
-    _failedSettingsRequests(0)
+    _settingsTimer(this)
 {
     _sockAddr.setObjectName("DomainServer");
 
     // if we get a socket that make sure our NetworkPeer ping timer stops
     connect(this, &DomainHandler::completedSocketDiscovery, &_icePeer, &NetworkPeer::stopPingTimer);
+    
+    // setup a timeout for failure on settings requests
+    static const int DOMAIN_SETTINGS_TIMEOUT_MS = 5000;
+    _settingsTimer.setInterval(DOMAIN_SETTINGS_TIMEOUT_MS);
+    connect(&_settingsTimer, &QTimer::timeout, this, &DomainHandler::settingsReceiveFail);
 }
 
 void DomainHandler::disconnect() {
@@ -80,13 +85,16 @@ void DomainHandler::sendDisconnectPacket() {
 
 void DomainHandler::clearSettings() {
     _settingsObject = QJsonObject();
-    _failedSettingsRequests = 0;
 }
 
 void DomainHandler::softReset() {
     qCDebug(networking) << "Resetting current domain connection information.";
     disconnect();
+    
     clearSettings();
+    
+    // cancel the failure timeout for any pending requests for settings
+    QMetaObject::invokeMethod(&_settingsTimer, "stop", Qt::AutoConnection);
 }
 
 void DomainHandler::hardReset() {
@@ -250,34 +258,35 @@ void DomainHandler::requestDomainSettings() {
     
     NodeType_t owningNodeType = DependencyManager::get<NodeList>()->getOwnerType();
     if (owningNodeType == NodeType::Agent) {
-        // for now the agent nodes don't need any settings - this allows local assignment-clients
-        // to connect to a domain that is using automatic networking (since we don't have TCP hole punch yet)
+        // for now the agent nodes don't need any domain settings
         _settingsObject = QJsonObject();
         emit settingsReceived(_settingsObject);
     } else {
-        if (_settingsObject.isEmpty()) {
-            qCDebug(networking) << "Requesting settings from domain server";
+        qCDebug(networking) << "Requesting settings from domain server";
         
-            Assignment::Type assignmentType = Assignment::typeForNodeType(DependencyManager::get<NodeList>()->getOwnerType());
+        Assignment::Type assignmentType = Assignment::typeForNodeType(DependencyManager::get<NodeList>()->getOwnerType());
         
-            auto packet = NLPacket::create(PacketType::DomainSettingsRequest, sizeof(assignmentType), true, false);
-            packet->writePrimitive(assignmentType);
+        auto packet = NLPacket::create(PacketType::DomainSettingsRequest, sizeof(assignmentType), true, false);
+        packet->writePrimitive(assignmentType);
         
-            auto nodeList = DependencyManager::get<LimitedNodeList>();
-            nodeList->sendPacket(std::move(packet), _sockAddr);
-        }
+        auto nodeList = DependencyManager::get<LimitedNodeList>();
+        nodeList->sendPacket(std::move(packet), _sockAddr);
+        
+        _settingsTimer.start();
     }
 }
 
 void DomainHandler::processSettingsPacketList(QSharedPointer<NLPacketList> packetList) {
+    // stop our settings timer since we successfully requested the settings we need
+    _settingsTimer.stop();
+    
     auto data = packetList->getMessage();
 
     _settingsObject = QJsonDocument::fromJson(data).object();
     
-    qCDebug(networking) << "Received domain settings: \n" << QString(data);
-
-    // reset failed settings requests to 0, we got them
-    _failedSettingsRequests = 0;
+    if (!_settingsObject.isEmpty()) {
+        qCDebug(networking) << "Received domain settings: \n" << _settingsObject;
+    }
 
     emit settingsReceived(_settingsObject);
 }

libraries/networking/src/DomainHandler.h

@@ -127,8 +127,8 @@ private:
     NetworkPeer _icePeer;
     bool _isConnected;
     QJsonObject _settingsObject;
-    int _failedSettingsRequests;
     QString _pendingPath;
+    QTimer _settingsTimer;
 };
 
 #endif // hifi_DomainHandler_h

libraries/networking/src/ThreadedAssignment.cpp

@@ -77,6 +77,9 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy
     connect(_domainServerTimer, SIGNAL(timeout()), this, SLOT(checkInWithDomainServerOrExit()));
     _domainServerTimer->start(DOMAIN_SERVER_CHECK_IN_MSECS);
     
+    // send a domain-server check in immediately
+    checkInWithDomainServerOrExit();
+    
     // move the domain server time to the NL so check-ins fire from there
     _domainServerTimer->moveToThread(nodeList->thread());
 
@@ -130,3 +133,8 @@ void ThreadedAssignment::checkInWithDomainServerOrExit() {
         DependencyManager::get<NodeList>()->sendDomainServerCheckIn();
     }
 }
+
+void ThreadedAssignment::domainSettingsRequestFailed() {
+    qDebug() << "Failed to retreive settings object from domain-server. Bailing on assignment.";
+    setFinished(true);
+}

libraries/networking/src/ThreadedAssignment.h

@@ -41,6 +41,9 @@ protected:
     QTimer* _domainServerTimer = nullptr;
     QTimer* _statsTimer = nullptr;
     
+protected slots:
+    void domainSettingsRequestFailed();
+    
 private slots:
     void startSendingStats();
     void stopSendingStats();

libraries/networking/src/udt/SendQueue.cpp

@@ -395,7 +395,7 @@ bool SendQueue::isInactive(bool sentAPacket) {
             
 #ifdef UDT_CONNECTION_DEBUG
             qCDebug(networking) << "SendQueue to" << _destination << "reached" << NUM_TIMEOUTS_BEFORE_INACTIVE << "timeouts"
-            << "and 5s before receiving any ACK/NAK and is now inactive. Stopping.";
+                << "and 5s before receiving any ACK/NAK and is now inactive. Stopping.";
 #endif
             
             deactivate();
@@ -427,9 +427,9 @@ bool SendQueue::isInactive(bool sentAPacket) {
                 if (cvStatus == std::cv_status::timeout) {
 #ifdef UDT_CONNECTION_DEBUG
                     qCDebug(networking) << "SendQueue to" << _destination << "has been empty for"
-                    << EMPTY_QUEUES_INACTIVE_TIMEOUT.count()
-                    << "seconds and receiver has ACKed all packets."
-                    << "The queue is now inactive and will be stopped.";
+                        << EMPTY_QUEUES_INACTIVE_TIMEOUT.count()
+                        << "seconds and receiver has ACKed all packets."
+                        << "The queue is now inactive and will be stopped.";
 #endif
                     
                     // Deactivate queue