Merge pull request #13038 from highfidelity/stable

Merge stable to RC66.2

assignment-client/src/avatars/AvatarMixerClientData.h

@@ -115,11 +115,7 @@ public:
     uint64_t getLastOtherAvatarEncodeTime(QUuid otherAvatar) const;
     void setLastOtherAvatarEncodeTime(const QUuid& otherAvatar, const uint64_t& time);
 
-    QVector<JointData>& getLastOtherAvatarSentJoints(QUuid otherAvatar) {
+    QVector<JointData>& getLastOtherAvatarSentJoints(QUuid otherAvatar) { return _lastOtherAvatarSentJoints[otherAvatar]; }
-        auto& lastOtherAvatarSentJoints = _lastOtherAvatarSentJoints[otherAvatar];
-        lastOtherAvatarSentJoints.resize(_avatar->getJointCount());
-        return lastOtherAvatarSentJoints;
-    }
 
     void queuePacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer node);
     int processPackets(); // returns number of packets processed

assignment-client/src/avatars/AvatarMixerSlave.cpp

@@ -381,6 +381,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
         bool includeThisAvatar = true;
         auto lastEncodeForOther = nodeData->getLastOtherAvatarEncodeTime(otherNode->getUUID());
         QVector<JointData>& lastSentJointsForOther = nodeData->getLastOtherAvatarSentJoints(otherNode->getUUID());
+
+        lastSentJointsForOther.resize(otherAvatar->getJointCount());
+
         bool distanceAdjust = true;
         glm::vec3 viewerPosition = myPosition;
         AvatarDataPacket::HasFlags hasFlagsOut; // the result of the toByteArray

domain-server/src/DomainGatekeeper.cpp

@@ -451,11 +451,12 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect
         return SharedNodePointer();
     }
 
-    QUuid hintNodeID;
+    QUuid existingNodeID;
 
     // in case this is a node that's failing to connect
     // double check we don't have the same node whose sockets match exactly already in the list
     limitedNodeList->eachNodeBreakable([&](const SharedNodePointer& node){
+
         if (node->getPublicSocket() == nodeConnection.publicSockAddr && node->getLocalSocket() == nodeConnection.localSockAddr) {
             // we have a node that already has these exact sockets - this can occur if a node
             // is failing to connect to the domain
@@ -465,15 +466,20 @@ SharedNodePointer DomainGatekeeper::processAgentConnectRequest(const NodeConnect
             auto existingNodeData = static_cast<DomainServerNodeData*>(node->getLinkedData());
 
             if (existingNodeData->getUsername() == username) {
-                hintNodeID = node->getUUID();
+                qDebug() << "Deleting existing connection from same sockaddr: " << node->getUUID();
+                existingNodeID = node->getUUID();
                 return false;
             }
         }
         return true;
     });
 
+    if (!existingNodeID.isNull()) {
+        limitedNodeList->killNodeWithUUID(existingNodeID);
+    }
+
     // add the connecting node (or re-use the matched one from eachNodeBreakable above)
-    SharedNodePointer newNode = addVerifiedNodeFromConnectRequest(nodeConnection, hintNodeID);
+    SharedNodePointer newNode = addVerifiedNodeFromConnectRequest(nodeConnection);
 
     // set the edit rights for this user
     newNode->setPermissions(userPerms);

interface/src/Crashpad.cpp

@@ -31,10 +31,27 @@ using namespace crashpad;
 static const std::string BACKTRACE_URL { CMAKE_BACKTRACE_URL };
 static const std::string BACKTRACE_TOKEN { CMAKE_BACKTRACE_TOKEN };
 
+static std::wstring gIPCPipe;
+
 extern QString qAppFileName();
 
 // crashpad::AnnotationList* crashpadAnnotations { nullptr };
 
+#include <Windows.h>
+
+LONG WINAPI vectoredExceptionHandler(PEXCEPTION_POINTERS pExceptionInfo) {
+    if (pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_HEAP_CORRUPTION ||
+        pExceptionInfo->ExceptionRecord->ExceptionCode == STATUS_STACK_BUFFER_OVERRUN) {
+        CrashpadClient client;
+        if (gIPCPipe.length()) {
+            client.SetHandlerIPCPipe(gIPCPipe);
+        }
+        client.DumpAndCrash(pExceptionInfo);
+    }
+
+    return EXCEPTION_CONTINUE_SEARCH;
+}
+
 bool startCrashHandler() {
     if (BACKTRACE_URL.empty() || BACKTRACE_TOKEN.empty()) {
         return false;
@@ -76,7 +93,12 @@ bool startCrashHandler() {
     // Enable automated uploads.
     database->GetSettings()->SetUploadsEnabled(true);
 
-    return client.StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true);
+    bool result = client.StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true);
+    gIPCPipe = client.GetHandlerIPCPipe();
+
+    AddVectoredExceptionHandler(0, vectoredExceptionHandler);
+
+    return result;
 }
 
 void setCrashAnnotation(std::string name, std::string value) {

interface/src/Menu.cpp

@@ -749,32 +749,32 @@ Menu::Menu() {
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashPureVirtualFunction);
     connect(action, &QAction::triggered, qApp, []() { crash::pureVirtualCall(); });
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashPureVirtualFunctionThreaded);
-    connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::pureVirtualCall(); }); });
+    connect(action, &QAction::triggered, qApp, []() { std::thread(crash::pureVirtualCall).join(); });
 
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashDoubleFree);
     connect(action, &QAction::triggered, qApp, []() { crash::doubleFree(); });
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashDoubleFreeThreaded);
-    connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::doubleFree(); }); });
+    connect(action, &QAction::triggered, qApp, []() { std::thread(crash::doubleFree).join(); });
 
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashAbort);
     connect(action, &QAction::triggered, qApp, []() { crash::doAbort(); });
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashAbortThreaded);
-    connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::doAbort(); }); });
+    connect(action, &QAction::triggered, qApp, []() { std::thread(crash::doAbort).join(); });
 
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNullDereference);
     connect(action, &QAction::triggered, qApp, []() { crash::nullDeref(); });
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNullDereferenceThreaded);
-    connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::nullDeref(); }); });
+    connect(action, &QAction::triggered, qApp, []() { std::thread(crash::nullDeref).join(); });
 
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashOutOfBoundsVectorAccess);
     connect(action, &QAction::triggered, qApp, []() { crash::outOfBoundsVectorCrash(); });
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashOutOfBoundsVectorAccessThreaded);
-    connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::outOfBoundsVectorCrash(); }); });
+    connect(action, &QAction::triggered, qApp, []() { std::thread(crash::outOfBoundsVectorCrash).join(); });
 
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNewFault);
     connect(action, &QAction::triggered, qApp, []() { crash::newFault(); });
     action = addActionToQMenuAndActionHash(crashMenu, MenuOption::CrashNewFaultThreaded);
-    connect(action, &QAction::triggered, qApp, []() { std::thread([]() { crash::newFault(); }); });
+    connect(action, &QAction::triggered, qApp, []() { std::thread(crash::newFault).join(); });
 
     // Developer > Log...
     addActionToQMenuAndActionHash(developerMenu, MenuOption::Log, Qt::CTRL | Qt::SHIFT | Qt::Key_L,

libraries/networking/src/LimitedNodeList.cpp

@@ -578,9 +578,10 @@ void LimitedNodeList::reset() {
 
     // we need to make sure any socket connections are gone so wait on that here
     _nodeSocket.clearConnections();
+    _connectionIDs.clear();
 }
 
-bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID) {
+bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID, ConnectionID newConnectionID) {
     QReadLocker readLocker(&_nodeMutex);
 
     NodeHash::iterator it = _nodeHash.find(nodeUUID);
@@ -594,7 +595,7 @@ bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID) {
             _nodeHash.unsafe_erase(it);
         }
 
-        handleNodeKill(matchingNode);
+        handleNodeKill(matchingNode, newConnectionID);
         return true;
     }
 
@@ -609,7 +610,7 @@ void LimitedNodeList::processKillNode(ReceivedMessage& message) {
     killNodeWithUUID(nodeUUID);
 }
 
-void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) {
+void LimitedNodeList::handleNodeKill(const SharedNodePointer& node, ConnectionID nextConnectionID) {
     qCDebug(networking) << "Killed" << *node;
     node->stopPingTimer();
     emit nodeKilled(node);
@@ -617,6 +618,15 @@ void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) {
     if (auto activeSocket = node->getActiveSocket()) {
         _nodeSocket.cleanupConnection(*activeSocket);
     }
+
+    auto it = _connectionIDs.find(node->getUUID());
+    if (it != _connectionIDs.end()) {
+        if (nextConnectionID == NULL_CONNECTION_ID) {
+            it->second++;
+        } else {
+            it->second = nextConnectionID;
+        }
+    }
 }
 
 SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType,
@@ -638,6 +648,11 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t
 
         return matchingNode;
     } else {
+        auto it = _connectionIDs.find(uuid);
+        if (it == _connectionIDs.end()) {
+            _connectionIDs[uuid] = INITIAL_CONNECTION_ID;
+        }
+
         // we didn't have this node, so add them
         Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket);
         newNode->setIsReplicated(isReplicated);
@@ -712,13 +727,13 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t
     }
 }
 
-std::unique_ptr<NLPacket> LimitedNodeList::constructPingPacket(PingType_t pingType) {
+std::unique_ptr<NLPacket> LimitedNodeList::constructPingPacket(const QUuid& nodeId, PingType_t pingType) {
-    int packetSize = sizeof(PingType_t) + sizeof(quint64);
+    int packetSize = sizeof(PingType_t) + sizeof(quint64) + sizeof(int64_t);
 
     auto pingPacket = NLPacket::create(PacketType::Ping, packetSize);
-
     pingPacket->writePrimitive(pingType);
     pingPacket->writePrimitive(usecTimestampNow());
+    pingPacket->writePrimitive(_connectionIDs[nodeId]);
 
     return pingPacket;
 }

libraries/networking/src/LimitedNodeList.h

@@ -66,6 +66,10 @@ const QHostAddress DEFAULT_ASSIGNMENT_CLIENT_MONITOR_HOSTNAME = QHostAddress::Lo
 
 const QString USERNAME_UUID_REPLACEMENT_STATS_KEY = "$username";
 
+using ConnectionID = int64_t;
+const ConnectionID NULL_CONNECTION_ID { -1 };
+const ConnectionID INITIAL_CONNECTION_ID { 0 };
+
 typedef std::pair<QUuid, SharedNodePointer> UUIDNodePair;
 typedef tbb::concurrent_unordered_map<QUuid, SharedNodePointer, UUIDHasher> NodeHash;
 
@@ -180,7 +184,7 @@ public:
     void getPacketStats(float& packetsInPerSecond, float& bytesInPerSecond, float& packetsOutPerSecond, float& bytesOutPerSecond);
     void resetPacketStats();
 
-    std::unique_ptr<NLPacket> constructPingPacket(PingType_t pingType = PingType::Agnostic);
+    std::unique_ptr<NLPacket> constructPingPacket(const QUuid& nodeId, PingType_t pingType = PingType::Agnostic);
     std::unique_ptr<NLPacket> constructPingReplyPacket(ReceivedMessage& message);
 
     static std::unique_ptr<NLPacket> constructICEPingPacket(PingType_t pingType, const QUuid& iceID);
@@ -319,7 +323,7 @@ public slots:
     void startSTUNPublicSocketUpdate();
     virtual void sendSTUNRequest();
 
-    bool killNodeWithUUID(const QUuid& nodeUUID);
+    bool killNodeWithUUID(const QUuid& nodeUUID, ConnectionID newConnectionID = NULL_CONNECTION_ID);
 
 signals:
     void dataSent(quint8 channelType, int bytes);
@@ -371,7 +375,7 @@ protected:
     bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet, Node* sourceNode = nullptr);
     void processSTUNResponse(std::unique_ptr<udt::BasePacket> packet);
 
-    void handleNodeKill(const SharedNodePointer& node);
+    void handleNodeKill(const SharedNodePointer& node, ConnectionID newConnectionID = NULL_CONNECTION_ID);
 
     void stopInitialSTUNUpdate(bool success);
 
@@ -418,6 +422,7 @@ protected:
         }
     }
 
+    std::unordered_map<QUuid, ConnectionID> _connectionIDs;
 
 private slots:
     void flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp);

libraries/networking/src/NodeList.cpp

@@ -214,6 +214,20 @@ void NodeList::processPingPacket(QSharedPointer<ReceivedMessage> message, Shared
             sendingNode->setSymmetricSocket(senderSockAddr);
         }
     }
+
+    int64_t connectionID;
+
+    message->readPrimitive(&connectionID);
+
+    auto it = _connectionIDs.find(sendingNode->getUUID());
+    if (it != _connectionIDs.end()) {
+        if (connectionID > it->second) {
+            qDebug() << "Received a ping packet with a larger connection id (" << connectionID << ">" << it->second << ") from "
+                     << sendingNode->getUUID();
+            killNodeWithUUID(sendingNode->getUUID(), connectionID);
+        }
+    }
+
 }
 
 void NodeList::processPingReplyPacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode) {
@@ -705,16 +719,18 @@ void NodeList::pingPunchForInactiveNode(const SharedNodePointer& node) {
     if (node->getConnectionAttempts() > 0 && node->getConnectionAttempts() % NUM_DEBUG_CONNECTION_ATTEMPTS == 0) {
         qCDebug(networking) << "No response to UDP hole punch pings for node" << node->getUUID() << "in last second.";
     }
+    
+    auto nodeID = node->getUUID();
 
     // send the ping packet to the local and public sockets for this node
-    auto localPingPacket = constructPingPacket(PingType::Local);
+    auto localPingPacket = constructPingPacket(nodeID, PingType::Local);
     sendPacket(std::move(localPingPacket), *node, node->getLocalSocket());
 
-    auto publicPingPacket = constructPingPacket(PingType::Public);
+    auto publicPingPacket = constructPingPacket(nodeID, PingType::Public);
     sendPacket(std::move(publicPingPacket), *node, node->getPublicSocket());
 
     if (!node->getSymmetricSocket().isNull()) {
-        auto symmetricPingPacket = constructPingPacket(PingType::Symmetric);
+        auto symmetricPingPacket = constructPingPacket(nodeID, PingType::Symmetric);
         sendPacket(std::move(symmetricPingPacket), *node, node->getSymmetricSocket());
     }
 
@@ -784,7 +800,7 @@ void NodeList::sendKeepAlivePings() {
         auto type = node->getType();
         return !node->isUpstream() && _nodeTypesOfInterest.contains(type) && !NodeType::isDownstream(type);
     }, [&](const SharedNodePointer& node) {
-        sendPacket(constructPingPacket(), *node);
+        sendPacket(constructPingPacket(node->getUUID()), *node);
     });
 }
 

libraries/networking/src/ThreadedAssignment.cpp

@@ -84,7 +84,8 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy
     _domainServerTimer.start();
 
     // start sending stats packet once we connect to the domain
-    connect(&nodeList->getDomainHandler(), SIGNAL(connectedToDomain(const QString&)), &_statsTimer, SLOT(start()));
+    connect(&nodeList->getDomainHandler(), &DomainHandler::connectedToDomain,
+            &_statsTimer, static_cast<void (QTimer::*)()>(&QTimer::start));
 
     // stop sending stats if we disconnect
     connect(&nodeList->getDomainHandler(), &DomainHandler::disconnectedFromDomain, &_statsTimer, &QTimer::stop);

libraries/networking/src/udt/PacketHeaders.cpp

@@ -75,6 +75,8 @@ PacketVersion versionForPacketType(PacketType packetType) {
             return static_cast<PacketVersion>(IcePingVersion::SendICEPeerID);
         case PacketType::DomainSettings:
             return 18;  // replace min_avatar_scale and max_avatar_scale with min_avatar_height and max_avatar_height
+        case PacketType::Ping:
+            return static_cast<PacketVersion>(PingVersion::IncludeConnectionID);
         default:
             return 17;
     }

libraries/networking/src/udt/PacketHeaders.h

@@ -322,4 +322,8 @@ enum class IcePingVersion : PacketVersion {
     SendICEPeerID = 18
 };
 
+enum class PingVersion : PacketVersion {
+    IncludeConnectionID = 18
+};
+
 #endif // hifi_PacketHeaders_h

libraries/physics/src/ShapeFactory.cpp

@@ -114,6 +114,11 @@ btConvexHullShape* createConvexHull(const ShapeInfo::PointList& points) {
         minCorner = glm::min(minCorner, points[i]);
     }
     center /= (float)(points.size());
+    if (glm::any(glm::isnan(center))) {
+        // don't feed garbage to Bullet
+        assert(false); // crash here in DEBUG so we can investigate source of bad input
+        return nullptr;
+    }
 
     float margin = hull->getMargin();
 
@@ -265,7 +270,7 @@ btTriangleIndexVertexArray* createStaticMeshArray(const ShapeInfo& info) {
 }
 
 const btCollisionShape* ShapeFactory::createShapeFromInfo(const ShapeInfo& info) {
-    btCollisionShape* shape = NULL;
+    btCollisionShape* shape = nullptr;
     int type = info.getType();
     switch(type) {
         case SHAPE_TYPE_BOX: {

libraries/render/src/render/Item.cpp

@@ -118,9 +118,15 @@ uint32_t Item::fetchMetaSubItemBounds(ItemBounds& subItemBounds, Scene& scene) c
     auto numSubs = fetchMetaSubItems(subItems);
 
     for (auto id : subItems) {
-        auto& item = scene.getItem(id);
+        // TODO: Adding an extra check here even thought we shouldn't have too.
-        if (item.exist()) {
+        // We have cases when the id returned by fetchMetaSubItems is not allocated
-            subItemBounds.emplace_back(id, item.getBound());
+        if (scene.isAllocatedID(id)) {
+            auto& item = scene.getItem(id);
+            if (item.exist()) {
+                subItemBounds.emplace_back(id, item.getBound());
+            } else {
+                numSubs--;
+            }
         } else {
             numSubs--;
         }