From 9c658490378a0e8d50604098653cb59574e22f6c Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:04:32 -0700 Subject: [PATCH 01/28] Add ordered sending to Socket and Connection --- libraries/networking/src/udt/Connection.cpp | 64 ++++++++++++++++++++- libraries/networking/src/udt/Connection.h | 21 +++++++ libraries/networking/src/udt/Socket.cpp | 38 ++++++++++-- libraries/networking/src/udt/Socket.h | 7 +++ 4 files changed, 123 insertions(+), 7 deletions(-) diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index d8aee60432..85c3dbcbda 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -16,9 +16,12 @@ #include #include "../HifiSockAddr.h" +#include "../NetworkLogging.h" + #include "CongestionControl.h" #include "ControlPacket.h" #include "Packet.h" +#include "PacketList.h" #include "Socket.h" using namespace udt; @@ -79,7 +82,32 @@ SendQueue& Connection::getSendQueue() { void Connection::sendReliablePacket(std::unique_ptr packet) { Q_ASSERT_X(packet->isReliable(), "Connection::send", "Trying to send an unreliable packet reliably."); - getSendQueue().queuePacket(move(packet)); + getSendQueue().queuePacket(std::move(packet)); +} + +void Connection::sendReliablePacketList(std::unique_ptr packetList) { + Q_ASSERT_X(packetList->isReliable(), "Connection::send", "Trying to send an unreliable packet reliably."); + getSendQueue().queuePacketList(std::move(packetList)); +} + +void Connection::queueReceivedMessagePacket(std::unique_ptr packet) { + Q_ASSERT(packet->isPartOfMessage()); + + auto messageNumber = packet->getMessageNumber(); + PendingReceivedMessage& pendingMessage = _pendingReceivedMessages[messageNumber]; + + pendingMessage.enqueuePacket(std::move(packet)); + + if (pendingMessage.isComplete()) { + // All messages have been received, create PacketList + auto packetList = PacketList::fromReceivedPackets(std::move(pendingMessage._packets)); + + _pendingReceivedMessages.erase(messageNumber); + + if (_parentSocket) { + _parentSocket->messageReceived(std::move(packetList)); + } + } } void Connection::sync() { @@ -609,3 +637,37 @@ void Connection::updateCongestionControlAndSendQueue(std::function cong _stats.recordPacketSendPeriod(_congestionControl->_packetSendPeriod); _stats.recordCongestionWindowSize(_congestionControl->_congestionWindowSize); } + +void PendingReceivedMessage::enqueuePacket(std::unique_ptr packet) { + if (_isComplete) { + qCDebug(networking) << "UNEXPECTED: Received packet for a message that is already complete"; + return; + } + + if (packet->getPacketPosition() == Packet::PacketPosition::FIRST) { + _hasFirstSequenceNumber = true; + _firstSequenceNumber = packet->getSequenceNumber(); + } else if (packet->getPacketPosition() == Packet::PacketPosition::LAST) { + _hasLastSequenceNumber = true; + _lastSequenceNumber = packet->getSequenceNumber(); + } else if (packet->getPacketPosition() == Packet::PacketPosition::ONLY) { + _hasFirstSequenceNumber = true; + _hasLastSequenceNumber = true; + _firstSequenceNumber = packet->getSequenceNumber(); + _lastSequenceNumber = packet->getSequenceNumber(); + } + + _packets.push_back(std::move(packet)); + + if (_hasFirstSequenceNumber && _hasLastSequenceNumber) { + auto numPackets = udt::seqlen(_firstSequenceNumber, _lastSequenceNumber); + if (uint64_t(numPackets) == _packets.size()) { + _isComplete = true; + + // Sort packets by sequence number + _packets.sort([](std::unique_ptr& a, std::unique_ptr& b) { + return a->getSequenceNumber() < b->getSequenceNumber(); + }); + } + } +} diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 65e7fe74f0..307c90eda5 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -30,8 +30,24 @@ namespace udt { class CongestionControl; class ControlPacket; class Packet; +class PacketList; class Socket; +class PendingReceivedMessage { +public: + void enqueuePacket(std::unique_ptr packet); + bool isComplete() const { return _isComplete; } + + std::list> _packets; + +private: + bool _isComplete { false }; + bool _hasFirstSequenceNumber { false }; + bool _hasLastSequenceNumber { false }; + SequenceNumber _firstSequenceNumber; + SequenceNumber _lastSequenceNumber; +}; + class Connection : public QObject { Q_OBJECT public: @@ -43,12 +59,15 @@ public: ~Connection(); void sendReliablePacket(std::unique_ptr packet); + void sendReliablePacketList(std::unique_ptr packet); void sync(); // rate control method, fired by Socket for all connections on SYN interval // return indicates if this packet was a duplicate bool processReceivedSequenceNumber(SequenceNumber sequenceNumber, int packetSize, int payloadSize); void processControl(std::unique_ptr controlPacket); + + void queueReceivedMessagePacket(std::unique_ptr packet); ConnectionStats::Stats sampleStats() { return _stats.sample(); } @@ -117,6 +136,8 @@ private: std::unique_ptr _congestionControl; std::unique_ptr _sendQueue; + + std::map _pendingReceivedMessages; int _packetsSinceACK { 0 }; // The number of packets that have been received during the current ACK interval diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 530d268425..0aac7f8f99 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -20,6 +20,7 @@ #include "ControlPacket.h" #include "Packet.h" #include "../NLPacket.h" +#include "PacketList.h" using namespace udt; @@ -104,6 +105,23 @@ qint64 Socket::writePacket(std::unique_ptr packet, const HifiSockAddr& s return writePacket(*packet, sockAddr); } +qint64 Socket::writePacketList(std::unique_ptr packetList, const HifiSockAddr& sockAddr) { + if (packetList->isReliable()) { + // Reliable and Ordered + // Reliable and Unordered + findOrCreateConnection(sockAddr).sendReliablePacketList(move(packetList)); + return 0; + } + + // Unerliable and Unordered + qint64 totalBytesSent = 0; + while (!packetList->_packets.empty()) { + totalBytesSent += writePacket(packetList->takeFront(), sockAddr); + } + + return totalBytesSent; +} + qint64 Socket::writeDatagram(const char* data, qint64 size, const HifiSockAddr& sockAddr) { return writeDatagram(QByteArray::fromRawData(data, size), sockAddr); } @@ -126,7 +144,7 @@ qint64 Socket::writeDatagram(const QByteArray& datagram, const HifiSockAddr& soc Connection& Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { auto it = _connectionsHash.find(sockAddr); - + if (it == _connectionsHash.end()) { auto connection = std::unique_ptr(new Connection(this, sockAddr, _ccFactory->create())); it = _connectionsHash.insert(it, std::make_pair(sockAddr, std::move(connection))); @@ -135,6 +153,12 @@ Connection& Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { return *it->second; } +void Socket::messageReceived(std::unique_ptr packetList) { + if (_packetListHandler) { + _packetListHandler(std::move(packetList)); + } +} + void Socket::readPendingDatagrams() { int packetSizeWithHeader = -1; while ((packetSizeWithHeader = _udpSocket.pendingDatagramSize()) != -1) { @@ -177,16 +201,18 @@ void Socket::readPendingDatagrams() { // call our verification operator to see if this packet is verified if (!_packetFilterOperator || _packetFilterOperator(*packet)) { - if (packet->isReliable()) { // if this was a reliable packet then signal the matching connection with the sequence number auto& connection = findOrCreateConnection(senderSockAddr); connection.processReceivedSequenceNumber(packet->getSequenceNumber(), - packet->getDataSize(), - packet->getPayloadSize()); + packet->getDataSize(), + packet->getPayloadSize()); } - - if (_packetHandler) { + + if (packet->isPartOfMessage()) { + auto& connection = findOrCreateConnection(senderSockAddr); + connection.queueReceivedMessagePacket(std::move(packet)); + } else if (_packetHandler) { // call the verified packet callback to let it handle this packet _packetHandler(std::move(packet)); } diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index e3cf347905..0c4276a767 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -30,12 +30,14 @@ namespace udt { class BasePacket; class ControlSender; class Packet; +class PacketList; class SequenceNumber; using PacketFilterOperator = std::function; using BasePacketHandler = std::function)>; using PacketHandler = std::function)>; +using PacketListHandler = std::function)>; class Socket : public QObject { Q_OBJECT @@ -48,6 +50,7 @@ public: qint64 writeBasePacket(const BasePacket& packet, const HifiSockAddr& sockAddr); qint64 writePacket(const Packet& packet, const HifiSockAddr& sockAddr); qint64 writePacket(std::unique_ptr packet, const HifiSockAddr& sockAddr); + qint64 writePacketList(std::unique_ptr packetList, const HifiSockAddr& sockAddr); qint64 writeDatagram(const char* data, qint64 size, const HifiSockAddr& sockAddr); qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& sockAddr); @@ -56,6 +59,7 @@ public: void setPacketFilterOperator(PacketFilterOperator filterOperator) { _packetFilterOperator = filterOperator; } void setPacketHandler(PacketHandler handler) { _packetHandler = handler; } + void setPacketListHandler(PacketListHandler handler) { _packetListHandler = handler; } void addUnfilteredHandler(const HifiSockAddr& senderSockAddr, BasePacketHandler handler) { _unfilteredHandlers[senderSockAddr] = handler; } @@ -63,6 +67,8 @@ public: void setCongestionControlFactory(std::unique_ptr ccFactory); void connectToSendSignal(const HifiSockAddr& destinationAddr, QObject* receiver, const char* slot); + + void messageReceived(std::unique_ptr packetList); ConnectionStats::Stats sampleStatsForConnection(const HifiSockAddr& destination); std::vector getConnectionSockAddrs(); @@ -78,6 +84,7 @@ private: QUdpSocket _udpSocket { this }; PacketFilterOperator _packetFilterOperator; PacketHandler _packetHandler; + PacketListHandler _packetListHandler; std::unordered_map _unfilteredHandlers; std::unordered_map _unreliableSequenceNumbers; From 5cb028cf4382d575589507f180807533e0f992bb Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:05:07 -0700 Subject: [PATCH 02/28] Add support for PacketList to SendQueue --- libraries/networking/src/udt/SendQueue.cpp | 45 ++++++++++++++++++++++ libraries/networking/src/udt/SendQueue.h | 6 +++ 2 files changed, 51 insertions(+) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index d3095bff15..2c2ff7350b 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -20,6 +20,7 @@ #include "ControlPacket.h" #include "Packet.h" +#include "PacketList.h" #include "Socket.h" using namespace udt; @@ -63,6 +64,44 @@ void SendQueue::queuePacket(std::unique_ptr packet) { } } +void SendQueue::queuePacketList(std::unique_ptr packetList) { + Q_ASSERT(packetList->_packets.size() > 0); + + { + QWriteLocker locker(&_packetsLock); + + auto messageNumber = getNextMessageNumber(); + + if (packetList->_packets.size() == 1) { + auto packet = packetList->takeFront(); + packet->setPacketPosition(Packet::PacketPosition::ONLY); + + packet->writeMessageNumber(messageNumber); + _packets.push_back(std::move(packet)); + } else { + bool haveMarkedFirstPacket = false; + while (!packetList->_packets.empty()) { + auto packet = packetList->takeFront(); + if (!haveMarkedFirstPacket) { + packet->setPacketPosition(Packet::PacketPosition::FIRST); + haveMarkedFirstPacket = true; + } else if (packetList->_packets.empty()) { + packet->setPacketPosition(Packet::PacketPosition::LAST); + } else { + packet->setPacketPosition(Packet::PacketPosition::MIDDLE); + } + + packet->writeMessageNumber(messageNumber); + + _packets.push_back(std::move(packet)); + } + } + } + if (!this->thread()->isRunning()) { + this->thread()->start(); + } +} + void SendQueue::stop() { _isRunning = false; } @@ -121,6 +160,12 @@ SequenceNumber SendQueue::getNextSequenceNumber() { return _currentSequenceNumber; } +uint32_t SendQueue::getNextMessageNumber() { + static const MessageNumber MAX_MESSAGE_NUMBER = MessageNumber(1) << 30; + _currentMessageNumber = (_currentMessageNumber + 1) % MAX_MESSAGE_NUMBER; + return _currentMessageNumber; +} + void SendQueue::sendNewPacketAndAddToSentList(std::unique_ptr newPacket, SequenceNumber sequenceNumber) { // write the sequence number and send the packet newPacket->writeSequenceNumber(sequenceNumber); diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index bbb1dc3798..901a9f7a87 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -31,7 +31,10 @@ namespace udt { class BasePacket; class ControlPacket; class Packet; +class PacketList; class Socket; + +using MessageNumber = uint32_t; class SendQueue : public QObject { Q_OBJECT @@ -40,6 +43,7 @@ public: static std::unique_ptr create(Socket* socket, HifiSockAddr destination); void queuePacket(std::unique_ptr packet); + void queuePacketList(std::unique_ptr packetList); int getQueueSize() const { QReadLocker locker(&_packetsLock); return _packets.size(); } SequenceNumber getCurrentSequenceNumber() const { return SequenceNumber(_atomicCurrentSequenceNumber); } @@ -73,6 +77,7 @@ private: // Increments current sequence number and return it SequenceNumber getNextSequenceNumber(); + MessageNumber getNextMessageNumber(); mutable QReadWriteLock _packetsLock; // Protects the packets to be sent list. std::list> _packets; // List of packets to be sent @@ -82,6 +87,7 @@ private: std::atomic _lastACKSequenceNumber; // Last ACKed sequence number + MessageNumber _currentMessageNumber { 0 }; SequenceNumber _currentSequenceNumber; // Last sequence number sent out std::atomic _atomicCurrentSequenceNumber;// Atomic for last sequence number sent out From 8032f05ed6666b5878d94464ff5200b33b7772c1 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:06:30 -0700 Subject: [PATCH 03/28] Add support for generating PacketList from received data --- libraries/networking/src/udt/PacketList.cpp | 54 +++++++++++++++++++-- libraries/networking/src/udt/PacketList.h | 27 +++++++++-- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index 494587971b..e5386a4cc9 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -13,17 +13,26 @@ #include -#include "Packet.h" +#include "../NLPacket.h" using namespace udt; -PacketList::PacketList(PacketType packetType, QByteArray extendedHeader) : +PacketList::PacketList(PacketType packetType, QByteArray extendedHeader, bool isReliable, bool isOrdered) : _packetType(packetType), + _isReliable(isReliable), + _isOrdered(isOrdered), _extendedHeader(extendedHeader) { + Q_ASSERT_X(!(!_isReliable && _isOrdered), "PacketList", "Unreliable ordered PacketLists are not currently supported"); QIODevice::open(WriteOnly); } +PacketList::PacketList(PacketList&& other) : + _packetType(other._packetType), + _packets(std::move(other._packets)) +{ +} + void PacketList::startSegment() { _segmentStartIndex = _currentPacket ? _currentPacket->pos() : _extendedHeader.size(); } @@ -32,10 +41,30 @@ void PacketList::endSegment() { _segmentStartIndex = -1; } +size_t PacketList::getDataSize() const { + size_t totalBytes = 0; + for (const auto& packet : _packets) { + totalBytes += packet->getDataSize(); + } + + if (_currentPacket) { + totalBytes += _currentPacket->getDataSize(); + } + + return totalBytes; +} + +std::unique_ptr PacketList::fromReceivedPackets(std::list>&& packets) { + auto packetList = std::unique_ptr(new PacketList(PacketType::Unknown, QByteArray(), true, true)); + packetList->_packets = std::move(packets); + return packetList; +} + std::unique_ptr PacketList::createPacket() { // use the static create method to create a new packet - // TODO: create a packet with correct reliability and messaging - return Packet::create(); + // If this packet list is supposed to be ordered then we consider this to be part of a message + bool isPartOfMessage = _isOrdered; + return Packet::create(-1, _isReliable, isPartOfMessage); } std::unique_ptr PacketList::createPacketWithExtendedHeader() { @@ -53,6 +82,23 @@ std::unique_ptr PacketList::createPacketWithExtendedHeader() { return packet; } +QByteArray PacketList::getAllData() { + size_t sizeBytes = 0; + + for (const auto& packet : _packets) { + sizeBytes += packet->size(); + } + + QByteArray data; + data.reserve(sizeBytes); + + for (auto& packet : _packets) { + data.append(packet->getPayload(), packet->getPayloadSize()); + } + + return data; +} + qint64 PacketList::writeData(const char* data, qint64 maxSize) { if (!_currentPacket) { // we don't have a current packet, time to set one up diff --git a/libraries/networking/src/udt/PacketList.h b/libraries/networking/src/udt/PacketList.h index 18acb7fa17..5596253a6c 100644 --- a/libraries/networking/src/udt/PacketList.h +++ b/libraries/networking/src/udt/PacketList.h @@ -16,6 +16,7 @@ #include +#include "Packet.h" #include "PacketHeaders.h" class LimitedNodeList; @@ -27,26 +28,44 @@ class Packet; class PacketList : public QIODevice { Q_OBJECT public: - PacketList(PacketType packetType, QByteArray extendedHeader = QByteArray()); + PacketList(PacketType packetType, QByteArray extendedHeader = QByteArray(), bool isReliable = false, bool isOrdered = false); + PacketList(PacketList&& other); + + static std::unique_ptr fromReceivedPackets(std::list>&& packets); virtual bool isSequential() const { return true; } + + bool isReliable() const { return _isReliable; } + bool isOrdered() const { return _isOrdered; } void startSegment(); void endSegment(); PacketType getType() const { return _packetType; } int getNumPackets() const { return _packets.size() + (_currentPacket ? 1 : 0); } + + QByteArray getExtendedHeader() const { return _extendedHeader; } + + size_t getDataSize() const; void closeCurrentPacket(bool shouldSendEmpty = false); - + + QByteArray getAllData(); + template qint64 readPrimitive(T* data); template qint64 writePrimitive(const T& data); + std::list> _packets; protected: virtual qint64 writeData(const char* data, qint64 maxSize); virtual qint64 readData(char* data, qint64 maxSize) { return 0; } + PacketType _packetType; + private: friend class ::LimitedNodeList; + friend class Socket; + friend class SendQueue; + friend class NLPacketList; PacketList(const PacketList& other) = delete; PacketList& operator=(const PacketList& other) = delete; @@ -58,11 +77,11 @@ private: virtual std::unique_ptr createPacket(); std::unique_ptr createPacketWithExtendedHeader(); - PacketType _packetType; + Packet::MessageNumber _messageNumber; + bool _isReliable = false; bool _isOrdered = false; std::unique_ptr _currentPacket; - std::list> _packets; int _segmentStartIndex = -1; From 46d5f73e4ebdc9715920092eb05e549cc4852f17 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:07:08 -0700 Subject: [PATCH 04/28] Add support for creating NLPacketList from PacketList --- libraries/networking/src/NLPacketList.cpp | 22 +++++++++++++++++----- libraries/networking/src/NLPacketList.h | 13 +++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/libraries/networking/src/NLPacketList.cpp b/libraries/networking/src/NLPacketList.cpp index df36228e7a..6dfd627271 100644 --- a/libraries/networking/src/NLPacketList.cpp +++ b/libraries/networking/src/NLPacketList.cpp @@ -11,15 +11,27 @@ #include "NLPacketList.h" -#include "NLPacket.h" +#include "udt/Packet.h" -NLPacketList::NLPacketList(PacketType packetType, QByteArray extendedHeader) : - PacketList(packetType, extendedHeader) +NLPacketList::NLPacketList(PacketType packetType, QByteArray extendedHeader, bool isReliable, bool isOrdered) : + PacketList(packetType, extendedHeader, isReliable, isOrdered) { +} +NLPacketList::NLPacketList(PacketList&& other) : PacketList(other.getType(), other.getExtendedHeader(), other.isReliable(), other.isOrdered()) { + // Update _packets + for (auto& packet : other._packets) { + auto nlPacket = NLPacket::fromBase(std::move(packet)); + _packets.push_back(std::move(nlPacket)); + } + + if (_packets.size() > 0) { + auto nlPacket = static_cast(_packets.front().get()); + _sourceID = nlPacket->getSourceID(); + _packetType = nlPacket->getType(); + } } std::unique_ptr NLPacketList::createPacket() { - return NLPacket::create(getType()); + return NLPacket::create(getType(), -1, isReliable(), isOrdered()); } - diff --git a/libraries/networking/src/NLPacketList.h b/libraries/networking/src/NLPacketList.h index 512ec23d22..cb48db08f2 100644 --- a/libraries/networking/src/NLPacketList.h +++ b/libraries/networking/src/NLPacketList.h @@ -12,17 +12,26 @@ #ifndef hifi_NLPacketList_h #define hifi_NLPacketList_h +#include + #include "udt/PacketList.h" +#include "NLPacket.h" + class NLPacketList : public udt::PacketList { public: - NLPacketList(PacketType packetType, QByteArray extendedHeader = QByteArray()); + NLPacketList(PacketType packetType, QByteArray extendedHeader = QByteArray(), bool isReliable = false, bool isOrdered = false); + NLPacketList(PacketList&& packetList); + + QUuid getSourceID() const { return _sourceID; } private: NLPacketList(const NLPacketList& other) = delete; NLPacketList& operator=(const NLPacketList& other) = delete; - + virtual std::unique_ptr createPacket(); + + QUuid _sourceID; }; #endif // hifi_PacketList_h From 69a2d0b5b03c30ccf933b31f3344277a36d391ef Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:07:54 -0700 Subject: [PATCH 05/28] Add 32-bit message data to packet header --- libraries/networking/src/udt/Packet.cpp | 33 ++++++++++++++++++++----- libraries/networking/src/udt/Packet.h | 18 +++++++++++++- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/libraries/networking/src/udt/Packet.cpp b/libraries/networking/src/udt/Packet.cpp index ae81341a1d..2cc0624f7b 100644 --- a/libraries/networking/src/udt/Packet.cpp +++ b/libraries/networking/src/udt/Packet.cpp @@ -57,7 +57,6 @@ Packet::Packet(qint64 size, bool isReliable, bool isPartOfMessage) : _isPartOfMessage(isPartOfMessage) { adjustPayloadStartAndCapacity(Packet::localHeaderSize(_isPartOfMessage)); - // set the UDT header to default values writeHeader(); } @@ -106,6 +105,12 @@ Packet& Packet::operator=(Packet&& other) { return *this; } +void Packet::writeMessageNumber(MessageNumber messageNumber) { + _isPartOfMessage = true; + _messageNumber = messageNumber; + writeHeader(); +} + void Packet::writeSequenceNumber(SequenceNumber sequenceNumber) const { _sequenceNumber = sequenceNumber; writeHeader(); @@ -115,14 +120,23 @@ static const uint32_t RELIABILITY_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BIT static const uint32_t MESSAGE_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BITS - 3); static const uint32_t BIT_FIELD_MASK = CONTROL_BIT_MASK | RELIABILITY_BIT_MASK | MESSAGE_BIT_MASK; +static const uint32_t PACKET_POSITION_MASK = uint32_t(0x03) << 30; +static const uint32_t MESSAGE_NUMBER_MASK = ~PACKET_POSITION_MASK; + void Packet::readHeader() const { - SequenceNumberAndBitField seqNumBitField = *reinterpret_cast(_packet.get()); + SequenceNumberAndBitField* seqNumBitField = reinterpret_cast(_packet.get()); - Q_ASSERT_X(!(seqNumBitField & CONTROL_BIT_MASK), "Packet::readHeader()", "This should be a data packet"); + Q_ASSERT_X(!(*seqNumBitField & CONTROL_BIT_MASK), "Packet::readHeader()", "This should be a data packet"); - _isReliable = (bool) (seqNumBitField & RELIABILITY_BIT_MASK); // Only keep reliability bit - _isPartOfMessage = (bool) (seqNumBitField & MESSAGE_BIT_MASK); // Only keep message bit - _sequenceNumber = SequenceNumber{ seqNumBitField & ~BIT_FIELD_MASK }; // Remove the bit field + _isReliable = (bool) (*seqNumBitField & RELIABILITY_BIT_MASK); // Only keep reliability bit + _isPartOfMessage = (bool) (*seqNumBitField & MESSAGE_BIT_MASK); // Only keep message bit + _sequenceNumber = SequenceNumber{ *seqNumBitField & ~BIT_FIELD_MASK }; // Remove the bit field + + if (_isPartOfMessage) { + MessageNumberAndBitField* messageNumberAndBitField = seqNumBitField + 1; + _messageNumber = *messageNumberAndBitField & MESSAGE_NUMBER_MASK; + _packetPosition = static_cast(*messageNumberAndBitField >> 30); + } } void Packet::writeHeader() const { @@ -140,5 +154,12 @@ void Packet::writeHeader() const { if (_isPartOfMessage) { *seqNumBitField |= MESSAGE_BIT_MASK; + + Q_ASSERT_X(!(_messageNumber & PACKET_POSITION_MASK), + "Packet::writeHeader()", "Message number is overflowing into bit field"); + + MessageNumberAndBitField* messageNumberAndBitField = seqNumBitField + 1; + *messageNumberAndBitField = _messageNumber; + *messageNumberAndBitField |= _packetPosition << 30; } } diff --git a/libraries/networking/src/udt/Packet.h b/libraries/networking/src/udt/Packet.h index 01c7fe90a1..2c57bbfc6c 100644 --- a/libraries/networking/src/udt/Packet.h +++ b/libraries/networking/src/udt/Packet.h @@ -31,6 +31,14 @@ public: // NOTE: The MessageNumber is only actually 29 bits to leave room for a bit field using MessageNumber = uint32_t; using MessageNumberAndBitField = uint32_t; + + // Use same size as MessageNumberAndBitField so we can use the enum with bitwise operations + enum PacketPosition : MessageNumberAndBitField { + ONLY = 0x0, + FIRST = 0x2, + MIDDLE = 0x3, + LAST = 0x1 + }; static std::unique_ptr create(qint64 size = -1, bool isReliable = false, bool isPartOfMessage = false); static std::unique_ptr fromReceivedPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); @@ -48,8 +56,14 @@ public: bool isPartOfMessage() const { return _isPartOfMessage; } bool isReliable() const { return _isReliable; } SequenceNumber getSequenceNumber() const { return _sequenceNumber; } + + MessageNumber getMessageNumber() const { return _messageNumber; } + + void setPacketPosition(PacketPosition position) { _packetPosition = position; } + PacketPosition getPacketPosition() const { return _packetPosition; } - void writeSequenceNumber(SequenceNumber sequenceNumber) const; + void writeMessageNumber(MessageNumber messageNumber); + void writeSequenceNumber(SequenceNumber sequenceNumber)const; protected: Packet(qint64 size, bool isReliable = false, bool isPartOfMessage = false); @@ -70,6 +84,8 @@ private: mutable bool _isReliable { false }; mutable bool _isPartOfMessage { false }; mutable SequenceNumber _sequenceNumber; + mutable PacketPosition _packetPosition { PacketPosition::ONLY }; + mutable MessageNumber _messageNumber { 0 }; }; } // namespace udt From 9154067cfb20d04015d51fee690ac874701ad032 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:08:25 -0700 Subject: [PATCH 06/28] Add PacketList interface to PacketReceiver --- libraries/networking/src/PacketReceiver.cpp | 156 +++++++++++++++++++- libraries/networking/src/PacketReceiver.h | 4 + 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index b3d591d875..1056c54c5e 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -89,6 +89,28 @@ void PacketReceiver::registerDirectListenerForTypes(const QSet& type } } +bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, const char* slot) { + QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); + + if (matchingMethod.isValid()) { + //registerVerifiedListener(type, listener, matchingMethod); + _packetListenerLock.lock(); + + if (_packetListListenerMap.contains(type)) { + qDebug() << "Warning: Registering a packet listener for packet type" << type + << "that will remove a previously registered listener"; + } + + // add the mapping + _packetListListenerMap[type] = ObjectMethodPair(QPointer(listener), matchingMethod); + + _packetListenerLock.unlock(); + return true; + } else { + return false; + } +} + bool PacketReceiver::registerListener(PacketType type, QObject* listener, const char* slot) { Q_ASSERT(listener); @@ -107,19 +129,25 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* // normalize the slot with the expected parameters - const QString NON_SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer"; + static const QString NON_SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer"; + static const QString NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS = "QSharedPointer"; QSet possibleSignatures { QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKET_LISTENER_PARAMETERS) }; + possibleSignatures << QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS); if (!NON_SOURCED_PACKETS.contains(type)) { static const QString SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer,QSharedPointer"; static const QString TYPEDEF_SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer,SharedNodePointer"; + static const QString SOURCED_PACKETLIST_LISTENER_PARAMETERS = "QSharedPointer,QSharedPointer"; + static const QString TYPEDEF_SOURCED_PACKETLIST_LISTENER_PARAMETERS = "QSharedPointer,SharedNodePointer"; // a sourced packet must take the shared pointer to the packet but optionally could include // a shared pointer to the node possibleSignatures << QString("%1(%2)").arg(slot).arg(TYPEDEF_SOURCED_PACKET_LISTENER_PARAMETERS); possibleSignatures << QString("%1(%2)").arg(slot).arg(SOURCED_PACKET_LISTENER_PARAMETERS); + possibleSignatures << QString("%1(%2)").arg(slot).arg(TYPEDEF_SOURCED_PACKETLIST_LISTENER_PARAMETERS); + possibleSignatures << QString("%1(%2)").arg(slot).arg(SOURCED_PACKETLIST_LISTENER_PARAMETERS); } int methodIndex = -1; @@ -190,6 +218,132 @@ void PacketReceiver::unregisterListener(QObject* listener) { _directConnectSetMutex.unlock(); } +void PacketReceiver::handleVerifiedPacketList(std::unique_ptr packetList) { + // if we're supposed to drop this packet then break out here + if (_shouldDropPackets) { + return; + } + + // setup an NLPacketList from the PacketList we were passed + auto nlPacketList = new NLPacketList(std::move(*packetList)); + + auto nodeList = DependencyManager::get(); + + _inPacketCount += nlPacketList->getNumPackets(); + _inByteCount += nlPacketList->getDataSize(); + + SharedNodePointer matchingNode; + + if (!nlPacketList->getSourceID().isNull()) { + matchingNode = nodeList->nodeWithUUID(nlPacketList->getSourceID()); + + if (matchingNode) { + // No matter if this packet is handled or not, we update the timestamp for the last time we heard + // from this sending node + matchingNode->setLastHeardMicrostamp(usecTimestampNow()); + } + } + + _packetListenerLock.lock(); + + bool listenerIsDead = false; + + auto it = _packetListListenerMap.find(nlPacketList->getType()); + + if (it != _packetListListenerMap.end() && it->second.isValid()) { + + auto listener = it.value(); + + if (listener.first) { + + bool success = false; + + // check if this is a directly connected listener + _directConnectSetMutex.lock(); + + Qt::ConnectionType connectionType = + _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; + + _directConnectSetMutex.unlock(); + + PacketType packetType = nlPacketList->getType(); + + if (matchingNode) { + emit dataReceived(matchingNode->getType(), nlPacketList->getDataSize()); + QMetaMethod metaMethod = listener.second; + + static const QByteArray QSHAREDPOINTER_NODE_NORMALIZED = QMetaObject::normalizedType("QSharedPointer"); + static const QByteArray SHARED_NODE_NORMALIZED = QMetaObject::normalizedType("SharedNodePointer"); + + // one final check on the QPointer before we go to invoke + if (listener.first) { + if (metaMethod.parameterTypes().contains(SHARED_NODE_NORMALIZED)) { + success = metaMethod.invoke(listener.first, + connectionType, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacketList)), + Q_ARG(SharedNodePointer, matchingNode)); + + } else if (metaMethod.parameterTypes().contains(QSHAREDPOINTER_NODE_NORMALIZED)) { + success = metaMethod.invoke(listener.first, + connectionType, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacketList)), + Q_ARG(QSharedPointer, matchingNode)); + + } else { + success = metaMethod.invoke(listener.first, + connectionType, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacketList))); + } + } else { + listenerIsDead = true; + } + } else { + emit dataReceived(NodeType::Unassigned, nlPacketList->getDataSize()); + + // one final check on the QPointer before we invoke + if (listener.first) { + success = listener.second.invoke(listener.first, + Q_ARG(QSharedPointer, + QSharedPointer(nlPacketList))); + } else { + listenerIsDead = true; + } + + } + + if (!success) { + qDebug().nospace() << "Error delivering packet " << packetType << " to listener " + << listener.first << "::" << qPrintable(listener.second.methodSignature()); + } + + } else { + listenerIsDead = true; + } + + if (listenerIsDead) { + qDebug().nospace() << "Listener for packet " << nlPacketList->getType() + << " has been destroyed. Removing from listener map."; + it = _packetListListenerMap.erase(it); + + // if it exists, remove the listener from _directlyConnectedObjects + _directConnectSetMutex.lock(); + _directlyConnectedObjects.remove(listener.first); + _directConnectSetMutex.unlock(); + } + + } else if (it == _packetListListenerMap.end()) { + qWarning() << "No listener found for packet type" << nlPacketList->getType(); + + // insert a dummy listener so we don't print this again + _packetListListenerMap.insert(nlPacketList->getType(), { nullptr, QMetaMethod() }); + } + + _packetListenerLock.unlock(); +} + void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { // if we're supposed to drop this packet then break out here diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index 531a8e60be..b5a4501476 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -21,6 +21,7 @@ #include #include "NLPacket.h" +#include "NLPacketList.h" #include "udt/PacketHeaders.h" class EntityEditPacketSender; @@ -42,10 +43,12 @@ public: void resetCounters() { _inPacketCount = 0; _inByteCount = 0; } bool registerListenerForTypes(const QSet& types, QObject* listener, const char* slot); + bool registerMessageListener(PacketType types, QObject* listener, const char* slot); bool registerListener(PacketType type, QObject* listener, const char* slot); void unregisterListener(QObject* listener); void handleVerifiedPacket(std::unique_ptr packet); + void handleVerifiedPacketList(std::unique_ptr packetList); signals: void dataReceived(quint8 channelType, int bytes); @@ -63,6 +66,7 @@ private: QMutex _packetListenerLock; QHash _packetListenerMap; + QHash _packetListListenerMap; int _inPacketCount = 0; int _inByteCount = 0; bool _shouldDropPackets = false; From 0b3986ef9ba333da822c10ff71659b03fc923477 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:09:02 -0700 Subject: [PATCH 07/28] Replace NLPacket unique_ptr ctor with && ctor --- libraries/networking/src/NLPacket.cpp | 6 +++--- libraries/networking/src/NLPacket.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 39f4f9c541..575a2c7a9c 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -54,7 +54,7 @@ std::unique_ptr NLPacket::fromBase(std::unique_ptr packet) { Q_ASSERT(packet); // call our constructor to create an NLPacket from this Packet - return std::unique_ptr(new NLPacket(std::move(packet))); + return std::unique_ptr(new NLPacket(std::move(*packet))); } std::unique_ptr NLPacket::createCopy(const NLPacket& other) { @@ -71,8 +71,8 @@ NLPacket::NLPacket(PacketType type, qint64 size, bool isReliable, bool isPartOfM writeTypeAndVersion(); } -NLPacket::NLPacket(std::unique_ptr packet) : - Packet(std::move(*packet.release())) +NLPacket::NLPacket(Packet&& packet) : + Packet(std::move(packet)) { readType(); readVersion(); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index c25ff198a5..b5d5fc0766 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -63,10 +63,10 @@ protected: NLPacket(PacketType type, qint64 size = -1, bool forceReliable = false, bool isPartOfMessage = false); NLPacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); - NLPacket(std::unique_ptr packet); NLPacket(const NLPacket& other); NLPacket(NLPacket&& other); + NLPacket(Packet&& other); NLPacket& operator=(const NLPacket& other); NLPacket& operator=(NLPacket&& other); From 62eaaed9e5abfe96490ff095c4b2bbf5cb0b3c90 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:10:05 -0700 Subject: [PATCH 08/28] Add PacketList support to LimitedNodeList --- libraries/networking/src/LimitedNodeList.cpp | 12 ++++++++++++ libraries/networking/src/LimitedNodeList.h | 1 + 2 files changed, 13 insertions(+) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 5f547ecaf3..e2bae6b5e8 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -95,6 +95,7 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short // set &PacketReceiver::handleVerifiedPacket as the verified packet callback for the udt::Socket using std::placeholders::_1; _nodeSocket.setPacketHandler(std::bind(&PacketReceiver::handleVerifiedPacket, _packetReceiver, _1)); + _nodeSocket.setPacketListHandler(std::bind(&PacketReceiver::handleVerifiedPacketList, _packetReceiver, _1)); // set our isPacketVerified method as the verify operator for the udt::Socket _nodeSocket.setPacketFilterOperator(std::bind(&LimitedNodeList::isPacketVerified, this, _1)); @@ -258,6 +259,7 @@ void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& conn } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode) { + Q_ASSERT(!packet.isPartOfMessage()); if (!destinationNode.getActiveSocket()) { return 0; } @@ -267,6 +269,7 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, const QUuid& connectionSecret) { + Q_ASSERT(!packet.isPartOfMessage()); Q_ASSERT_X(!packet.isReliable(), "LimitedNodeList::sendUnreliablePacket", "Trying to send a reliable packet unreliably."); @@ -277,6 +280,7 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiS } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& destinationNode) { + Q_ASSERT(!packet->isPartOfMessage()); if (!destinationNode.getActiveSocket()) { return 0; } @@ -286,6 +290,7 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, const QUuid& connectionSecret) { + Q_ASSERT(!packet->isPartOfMessage()); if (packet->isReliable()) { collectPacketStats(*packet); fillPacketHeader(*packet, connectionSecret); @@ -332,6 +337,13 @@ qint64 LimitedNodeList::sendPacketList(NLPacketList& packetList, const HifiSockA return bytesSent; } +qint64 LimitedNodeList::sendPacketList(std::unique_ptr packetList, const HifiSockAddr& sockAddr) { + // close the last packet in the list + packetList->closeCurrentPacket(); + + return _nodeSocket.writePacketList(std::move(packetList), sockAddr); +} + qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& destinationNode, const HifiSockAddr& overridenSockAddr) { // use the node's active socket as the destination socket if there is no overriden socket address diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 9c3ff058fc..5371831e3e 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -127,6 +127,7 @@ public: qint64 sendPacketList(NLPacketList& packetList, const Node& destinationNode); qint64 sendPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, const QUuid& connectionSecret = QUuid()); + qint64 sendPacketList(std::unique_ptr packetList, const HifiSockAddr& sockAddr); void (*linkedDataCreateCallback)(Node *); From 63d0205d1e9093d6f24b35b577337ea7fd6d8349 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:11:33 -0700 Subject: [PATCH 09/28] Move domain settings retrieval to UDT --- domain-server/src/DomainServer.cpp | 3 ++ .../src/DomainServerSettingsManager.cpp | 18 ++++++++++ .../src/DomainServerSettingsManager.h | 6 ++++ libraries/networking/src/Assignment.h | 2 +- libraries/networking/src/DomainHandler.cpp | 35 ++++++++++++------- libraries/networking/src/DomainHandler.h | 3 ++ libraries/networking/src/NodeList.cpp | 1 + .../networking/src/udt/PacketHeaders.cpp | 1 + libraries/networking/src/udt/PacketHeaders.h | 4 ++- 9 files changed, 58 insertions(+), 15 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index f15de0c458..ddbef90ef6 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -289,6 +289,9 @@ void DomainServer::setupNodeListAndAssignments(const QUuid& sessionUUID) { packetReceiver.registerListener(PacketType::ICEPing, this, "processICEPingPacket"); packetReceiver.registerListener(PacketType::ICEPingReply, this, "processICEPingReplyPacket"); packetReceiver.registerListener(PacketType::ICEServerPeerInformation, this, "processICEPeerInformationPacket"); + + // NodeList won't be available to the settings manager when it is created, so call registerListener here + packetReceiver.registerListener(PacketType::DomainSettingsRequest, &_settingsManager, "processSettingsRequestPacket"); // add whatever static assignments that have been parsed to the queue addStaticAssignmentsToQueue(); diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index 5cd5b5ef35..c9b52a17bb 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include "DomainServerSettingsManager.h" @@ -66,6 +67,23 @@ DomainServerSettingsManager::DomainServerSettingsManager() : QMetaObject::invokeMethod(QCoreApplication::instance(), "quit", Qt::QueuedConnection); } +void DomainServerSettingsManager::processSettingsRequestPacket(QSharedPointer packet) { + qDebug() << "Got request for domain settings"; + + Assignment::Type type; + packet->readPrimitive(&type); + + QJsonObject responseObject = responseObjectForType(QString::number(type)); + auto json = QJsonDocument(responseObject).toJson(); + + auto packetList = std::unique_ptr(new NLPacketList(PacketType::DomainSettings, QByteArray(), true, true)); + + packetList->write(json); + + auto nodeList = DependencyManager::get(); + nodeList->sendPacketList(std::move(packetList), packet->getSenderSockAddr()); +} + void DomainServerSettingsManager::setupConfigMap(const QStringList& argumentList) { _configMap.loadMasterAndUserConfig(argumentList); diff --git a/domain-server/src/DomainServerSettingsManager.h b/domain-server/src/DomainServerSettingsManager.h index 21bf099ef0..321f7b7214 100644 --- a/domain-server/src/DomainServerSettingsManager.h +++ b/domain-server/src/DomainServerSettingsManager.h @@ -18,6 +18,8 @@ #include #include +#include + const QString SETTINGS_PATHS_KEY = "paths"; const QString SETTINGS_PATH = "/settings"; @@ -38,6 +40,10 @@ public: QVariantMap& getUserSettingsMap() { return _configMap.getUserConfig(); } QVariantMap& getSettingsMap() { return _configMap.getMergedConfig(); } + +private slots: + void processSettingsRequestPacket(QSharedPointer packet); + private: QJsonObject responseObjectForType(const QString& typeValue, bool isAuthenticated = false); void recurseJSONObjectAndOverwriteSettings(const QJsonObject& postedObject, QVariantMap& settingsVariant); diff --git a/libraries/networking/src/Assignment.h b/libraries/networking/src/Assignment.h index 67f861f850..0fadc78770 100644 --- a/libraries/networking/src/Assignment.h +++ b/libraries/networking/src/Assignment.h @@ -25,7 +25,7 @@ class Assignment : public NodeData { Q_OBJECT public: - enum Type { + enum Type : uint8_t { AudioMixerType = 0, AvatarMixerType = 1, AgentType = 2, diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index 6c6c258235..81b7ee6c55 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -17,7 +17,9 @@ #include "Assignment.h" #include "HifiSockAddr.h" #include "NodeList.h" +#include "udt/Packet.h" #include "udt/PacketHeaders.h" +#include "NLPacket.h" #include "SharedUtil.h" #include "UserActivityLogger.h" #include "NetworkLogging.h" @@ -39,7 +41,7 @@ DomainHandler::DomainHandler(QObject* parent) : _failedSettingsRequests(0) { _sockAddr.setObjectName("DomainServer"); - + // if we get a socket that make sure our NetworkPeer ping timer stops connect(this, &DomainHandler::completedSocketDiscovery, &_icePeer, &NetworkPeer::stopPingTimer); } @@ -233,21 +235,15 @@ void DomainHandler::requestDomainSettings() { emit settingsReceived(_settingsObject); } else { if (_settingsObject.isEmpty()) { - // setup the URL required to grab settings JSON - QUrl settingsJSONURL; - settingsJSONURL.setScheme("http"); - settingsJSONURL.setHost(_hostname); - settingsJSONURL.setPort(DOMAIN_SERVER_HTTP_PORT); - settingsJSONURL.setPath("/settings.json"); + qCDebug(networking) << "Requesting settings from domain server"; + Assignment::Type assignmentType = Assignment::typeForNodeType(DependencyManager::get()->getOwnerType()); - settingsJSONURL.setQuery(QString("type=%1").arg(assignmentType)); - qCDebug(networking) << "Requesting domain-server settings at" << settingsJSONURL.toString(); + auto packet = NLPacket::create(PacketType::DomainSettingsRequest, sizeof(assignmentType), true, false); + packet->writePrimitive(assignmentType); - QNetworkRequest settingsRequest(settingsJSONURL); - settingsRequest.setHeader(QNetworkRequest::UserAgentHeader, HIGH_FIDELITY_USER_AGENT); - QNetworkReply* reply = NetworkAccessManager::getInstance().get(settingsRequest); - connect(reply, &QNetworkReply::finished, this, &DomainHandler::settingsRequestFinished); + auto nodeList = DependencyManager::get(); + nodeList->sendPacket(std::move(packet), _sockAddr); } } } @@ -286,6 +282,19 @@ void DomainHandler::settingsRequestFinished() { settingsReply->deleteLater(); } +void DomainHandler::processSettingsPacketList(QSharedPointer packetList) { + auto data = packetList->getAllData(); + + _settingsObject = QJsonDocument::fromJson(data).object(); + + qCDebug(networking) << "Received domain settings: \n" << QString(data); + + // reset failed settings requests to 0, we got them + _failedSettingsRequests = 0; + + emit settingsReceived(_settingsObject); +} + void DomainHandler::processICEPingReplyPacket(QSharedPointer packet) { const HifiSockAddr& senderSockAddr = packet->getSenderSockAddr(); qCDebug(networking) << "Received reply from domain-server on" << senderSockAddr; diff --git a/libraries/networking/src/DomainHandler.h b/libraries/networking/src/DomainHandler.h index 7bb0582914..349b3934eb 100644 --- a/libraries/networking/src/DomainHandler.h +++ b/libraries/networking/src/DomainHandler.h @@ -22,6 +22,8 @@ #include "HifiSockAddr.h" #include "NetworkPeer.h" #include "NLPacket.h" +#include "NLPacketList.h" +#include "Node.h" const unsigned short DEFAULT_DOMAIN_SERVER_PORT = 40102; const unsigned short DEFAULT_DOMAIN_SERVER_DTLS_PORT = 40103; @@ -85,6 +87,7 @@ public slots: void setHostnameAndPort(const QString& hostname, quint16 port = DEFAULT_DOMAIN_SERVER_PORT); void setIceServerHostnameAndID(const QString& iceServerHostname, const QUuid& id); + void processSettingsPacketList(QSharedPointer packetList); void processICEPingReplyPacket(QSharedPointer packet); void processDTLSRequirementPacket(QSharedPointer dtlsRequirementPacket); void processICEResponsePacket(QSharedPointer icePacket); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index ab69d11bc3..000180cec7 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -95,6 +95,7 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned packetReceiver.registerListener(PacketType::ICEPing, this, "processICEPingPacket"); packetReceiver.registerListener(PacketType::DomainServerAddedNode, this, "processDomainServerAddedNode"); packetReceiver.registerListener(PacketType::DomainServerConnectionToken, this, "processDomainServerConnectionTokenPacket"); + packetReceiver.registerMessageListener(PacketType::DomainSettings, &_domainHandler, "processSettingsPacketList"); packetReceiver.registerListener(PacketType::ICEServerPeerInformation, &_domainHandler, "processICEResponsePacket"); packetReceiver.registerListener(PacketType::DomainServerRequireDTLS, &_domainHandler, "processDTLSRequirementPacket"); packetReceiver.registerListener(PacketType::ICEPingReply, &_domainHandler, "processICEPingReplyPacket"); diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 1c2a38a2a6..819a65dc26 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -26,6 +26,7 @@ const QSet NON_SOURCED_PACKETS = QSet() << PacketType::DomainList << PacketType::DomainConnectionDenied << PacketType::DomainServerPathQuery << PacketType::DomainServerPathResponse << PacketType::DomainServerAddedNode + << PacketType::DomainSettingsRequest << PacketType::DomainSettings << PacketType::ICEServerPeerInformation << PacketType::ICEServerQuery << PacketType::ICEServerHeartbeat << PacketType::ICEPing << PacketType::ICEPingReply << PacketType::AssignmentClientStatus << PacketType::StopNode; diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 58e0832800..35898a21f5 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -71,7 +71,9 @@ enum class PacketType : uint8_t { EntityAdd, EntityErase, EntityEdit, - DomainServerConnectionToken + DomainServerConnectionToken, + DomainSettingsRequest, + DomainSettings }; const int NUM_BYTES_MD5_HASH = 16; From 97bb36add4b30ef19dc43dd055108524e1e12c4c Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:11:47 -0700 Subject: [PATCH 10/28] Add constant for message line size --- libraries/networking/src/udt/Constants.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/networking/src/udt/Constants.h b/libraries/networking/src/udt/Constants.h index eaad77d03e..8c73a230df 100644 --- a/libraries/networking/src/udt/Constants.h +++ b/libraries/networking/src/udt/Constants.h @@ -26,6 +26,7 @@ namespace udt { static const int UDP_RECEIVE_BUFFER_SIZE_BYTES = 1048576; static const int DEFAULT_SYN_INTERVAL_USECS = 10 * 1000; static const int SEQUENCE_NUMBER_BITS = sizeof(SequenceNumber) * 8; + static const int MESSAGE_LINE_NUMBER_BITS = 32; static const uint32_t CONTROL_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BITS - 1); } From 74547777df0dd2d8698b414d99199abd688a80fe Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:12:09 -0700 Subject: [PATCH 11/28] Add basic test to send packetList in udt-test --- tools/udt-test/src/UDTTest.cpp | 52 ++++++++++++++++++++++++++-------- tools/udt-test/src/UDTTest.h | 3 +- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/tools/udt-test/src/UDTTest.cpp b/tools/udt-test/src/UDTTest.cpp index 1f55fd1665..0b65907e29 100644 --- a/tools/udt-test/src/UDTTest.cpp +++ b/tools/udt-test/src/UDTTest.cpp @@ -15,6 +15,7 @@ #include #include +#include #include @@ -42,6 +43,9 @@ const QCommandLineOption MAX_SEND_PACKETS { const QCommandLineOption UNRELIABLE_PACKETS { "unreliable", "send unreliable packets (default is reliable)" }; +const QCommandLineOption ORDERED_PACKETS { + "ordered", "send ordered packets (default is unordered)" +}; const QStringList CLIENT_STATS_TABLE_HEADERS { "Send Rate (P/s)", "Bandwidth (P/s)", "RTT(ms)", "CW (P)", "Send Period (us)", @@ -129,6 +133,10 @@ UDTTest::UDTTest(int& argc, char** argv) : if (_argumentParser.isSet(UNRELIABLE_PACKETS)) { _sendReliable = false; } + + if (_argumentParser.isSet(ORDERED_PACKETS)) { + _sendOrdered = true; + } if (!_target.isNull()) { sendInitialPackets(); @@ -151,7 +159,7 @@ void UDTTest::parseArguments() { _argumentParser.addOptions({ PORT_OPTION, TARGET_OPTION, PACKET_SIZE, MIN_PACKET_SIZE, MAX_PACKET_SIZE, - MAX_SEND_BYTES, MAX_SEND_PACKETS, UNRELIABLE_PACKETS + MAX_SEND_BYTES, MAX_SEND_PACKETS, UNRELIABLE_PACKETS, ORDERED_PACKETS }); if (!_argumentParser.parse(arguments())) { @@ -206,20 +214,40 @@ void UDTTest::sendPacket() { int randomPacketSize = rand() % _maxPacketSize + _minPacketSize; packetPayloadSize = randomPacketSize - udt::Packet::localHeaderSize(false); } - - auto newPacket = udt::Packet::create(packetPayloadSize, _sendReliable); - newPacket->setPayloadSize(packetPayloadSize); - - _totalQueuedBytes += newPacket->getDataSize(); - - // queue or send this packet by calling write packet on the socket for our target - if (_sendReliable) { - _socket.writePacket(std::move(newPacket), _target); + + if (_sendOrdered) { + static int call = 0; + call = (call + 1) % 4; + if (call == 0) { + auto packetList = std::unique_ptr(new udt::PacketList(PacketType::BulkAvatarData, QByteArray(), true, true)); + for (int i = 0; i < 4; i++) { + packetList->writePrimitive(0x1); + packetList->writePrimitive(0x2); + packetList->writePrimitive(0x3); + packetList->writePrimitive(0x4); + packetList->closeCurrentPacket(false); + } + _totalQueuedBytes += packetList->getDataSize(); + + _socket.writePacketList(std::move(packetList), _target); + } + _totalQueuedPackets += 4; } else { - _socket.writePacket(*newPacket, _target); + auto newPacket = udt::Packet::create(packetPayloadSize, _sendReliable); + newPacket->setPayloadSize(packetPayloadSize); + + _totalQueuedBytes += newPacket->getDataSize(); + + // queue or send this packet by calling write packet on the socket for our target + // if ( + if (_sendReliable) { + _socket.writePacket(std::move(newPacket), _target); + } else { + _socket.writePacket(*newPacket, _target); + } + ++_totalQueuedPackets; } - ++_totalQueuedPackets; } void UDTTest::sampleStats() { diff --git a/tools/udt-test/src/UDTTest.h b/tools/udt-test/src/UDTTest.h index 1fd1836cf9..28aa3d340e 100644 --- a/tools/udt-test/src/UDTTest.h +++ b/tools/udt-test/src/UDTTest.h @@ -45,7 +45,8 @@ private: int _maxSendBytes { -1 }; // the number of bytes to send to the target before stopping int _maxSendPackets { -1 }; // the number of packets to send to the target before stopping - bool _sendReliable { true }; // wether packets are sent reliably or unreliably + bool _sendReliable { true }; // whether packets are sent reliably or unreliably + bool _sendOrdered { false }; // whether to send ordered packets int _totalQueuedPackets { 0 }; // keeps track of the number of packets we have already queued int _totalQueuedBytes { 0 }; // keeps track of the number of bytes we have already queued From 92dadb437c838d30a51b2fe4ec2733194f6ea8dc Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 18 Aug 2015 23:15:47 -0700 Subject: [PATCH 12/28] Replace magic number '30' for number of bits in message number --- libraries/networking/src/udt/Constants.h | 1 + libraries/networking/src/udt/SendQueue.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Constants.h b/libraries/networking/src/udt/Constants.h index 8c73a230df..0152444f84 100644 --- a/libraries/networking/src/udt/Constants.h +++ b/libraries/networking/src/udt/Constants.h @@ -27,6 +27,7 @@ namespace udt { static const int DEFAULT_SYN_INTERVAL_USECS = 10 * 1000; static const int SEQUENCE_NUMBER_BITS = sizeof(SequenceNumber) * 8; static const int MESSAGE_LINE_NUMBER_BITS = 32; + static const int MESSAGE_NUMBER_BITS = 30; static const uint32_t CONTROL_BIT_MASK = uint32_t(1) << (SEQUENCE_NUMBER_BITS - 1); } diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 2c2ff7350b..650542dec5 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -161,7 +161,7 @@ SequenceNumber SendQueue::getNextSequenceNumber() { } uint32_t SendQueue::getNextMessageNumber() { - static const MessageNumber MAX_MESSAGE_NUMBER = MessageNumber(1) << 30; + static const MessageNumber MAX_MESSAGE_NUMBER = MessageNumber(1) << MESSAGE_NUMBER_BITS; _currentMessageNumber = (_currentMessageNumber + 1) % MAX_MESSAGE_NUMBER; return _currentMessageNumber; } From cda0aaf7cf4c2247b2e8a9c3ad8101cd2b0f48aa Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 08:54:24 -0700 Subject: [PATCH 13/28] Update PacketReceiver to use scoped locks --- libraries/networking/src/PacketReceiver.cpp | 29 ++++++++++----------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 1056c54c5e..adcee4e22f 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -12,6 +12,8 @@ #include "PacketReceiver.h" +#include + #include "DependencyManager.h" #include "NetworkLogging.h" #include "NodeList.h" @@ -93,8 +95,7 @@ bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); if (matchingMethod.isValid()) { - //registerVerifiedListener(type, listener, matchingMethod); - _packetListenerLock.lock(); + QMutexLocker(&_packetListenerLock); if (_packetListListenerMap.contains(type)) { qDebug() << "Warning: Registering a packet listener for packet type" << type @@ -104,7 +105,6 @@ bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, // add the mapping _packetListListenerMap[type] = ObjectMethodPair(QPointer(listener), matchingMethod); - _packetListenerLock.unlock(); return true; } else { return false; @@ -244,7 +244,7 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p } } - _packetListenerLock.lock(); + QMutexLocker packetListenerLocker(&_packetListenerLock); bool listenerIsDead = false; @@ -258,13 +258,13 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p bool success = false; + Qt::ConnectionType connectionType; // check if this is a directly connected listener - _directConnectSetMutex.lock(); - - Qt::ConnectionType connectionType = - _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; - - _directConnectSetMutex.unlock(); + { + QMutexLocker directConnectLocker(&_directConnectSetMutex); + + connectionType = _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; + } PacketType packetType = nlPacketList->getType(); @@ -329,9 +329,10 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p it = _packetListListenerMap.erase(it); // if it exists, remove the listener from _directlyConnectedObjects - _directConnectSetMutex.lock(); - _directlyConnectedObjects.remove(listener.first); - _directConnectSetMutex.unlock(); + { + QMutexLocker directConnectLocker(&_directConnectSetMutex); + _directlyConnectedObjects.remove(listener.first); + } } } else if (it == _packetListListenerMap.end()) { @@ -340,8 +341,6 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p // insert a dummy listener so we don't print this again _packetListListenerMap.insert(nlPacketList->getType(), { nullptr, QMetaMethod() }); } - - _packetListenerLock.unlock(); } void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { From c6a8ccd4909ccea7d53d106b3b648839a938113b Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 08:56:17 -0700 Subject: [PATCH 14/28] Replace usage of qDebug with qCDebug in PacketReceiver --- libraries/networking/src/PacketReceiver.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index adcee4e22f..ed65519e5b 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -98,7 +98,7 @@ bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, QMutexLocker(&_packetListenerLock); if (_packetListListenerMap.contains(type)) { - qDebug() << "Warning: Registering a packet listener for packet type" << type + qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } @@ -165,7 +165,7 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* } if (methodIndex < 0) { - qDebug() << "PacketReceiver::registerListener expected a slot with one of the following signatures:" + qCDebug(networking) << "PacketReceiver::registerListener expected a slot with one of the following signatures:" << possibleSignatures.toList() << "- but such a slot was not found." << "Could not complete listener registration for type" << type; } @@ -186,7 +186,7 @@ void PacketReceiver::registerVerifiedListener(PacketType type, QObject* object, _packetListenerLock.lock(); if (_packetListenerMap.contains(type)) { - qDebug() << "Warning: Registering a packet listener for packet type" << type + qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } @@ -315,7 +315,7 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p } if (!success) { - qDebug().nospace() << "Error delivering packet " << packetType << " to listener " + qCDebug(networking).nospace() << "Error delivering packet " << packetType << " to listener " << listener.first << "::" << qPrintable(listener.second.methodSignature()); } @@ -324,7 +324,7 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p } if (listenerIsDead) { - qDebug().nospace() << "Listener for packet " << nlPacketList->getType() + qCDebug(networking).nospace() << "Listener for packet " << nlPacketList->getType() << " has been destroyed. Removing from listener map."; it = _packetListListenerMap.erase(it); @@ -441,7 +441,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } if (!success) { - qDebug().nospace() << "Error delivering packet " << packetType << " to listener " + qCDebug(networking).nospace() << "Error delivering packet " << packetType << " to listener " << listener.first << "::" << qPrintable(listener.second.methodSignature()); } @@ -450,7 +450,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } if (listenerIsDead) { - qDebug().nospace() << "Listener for packet " << nlPacket->getType() + qCDebug(networking).nospace() << "Listener for packet " << nlPacket->getType() << " has been destroyed. Removing from listener map."; it = _packetListenerMap.erase(it); From 10cd315a10b380b0b18e89c8bc6e36b6360e2375 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 08:58:53 -0700 Subject: [PATCH 15/28] Fix QMutexLocker in PacketReceiver --- libraries/networking/src/PacketReceiver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index ed65519e5b..12f9b618bb 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -95,7 +95,7 @@ bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); if (matchingMethod.isValid()) { - QMutexLocker(&_packetListenerLock); + QMutexLocker locker(&_packetListenerLock); if (_packetListListenerMap.contains(type)) { qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type From 3c5483a00839d2e5acb907a27e6405dcbd288f21 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 08:59:05 -0700 Subject: [PATCH 16/28] Fix style issue with const --- libraries/networking/src/udt/Packet.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Packet.h b/libraries/networking/src/udt/Packet.h index 2c57bbfc6c..565fc24616 100644 --- a/libraries/networking/src/udt/Packet.h +++ b/libraries/networking/src/udt/Packet.h @@ -63,7 +63,7 @@ public: PacketPosition getPacketPosition() const { return _packetPosition; } void writeMessageNumber(MessageNumber messageNumber); - void writeSequenceNumber(SequenceNumber sequenceNumber)const; + void writeSequenceNumber(SequenceNumber sequenceNumber) const; protected: Packet(qint64 size, bool isReliable = false, bool isPartOfMessage = false); From f5aac5f086c81c390544ef63f546ce3f06999572 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 09:02:17 -0700 Subject: [PATCH 17/28] Remove unused include --- libraries/networking/src/udt/PacketList.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index e5386a4cc9..23060fecf4 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -13,8 +13,6 @@ #include -#include "../NLPacket.h" - using namespace udt; PacketList::PacketList(PacketType packetType, QByteArray extendedHeader, bool isReliable, bool isOrdered) : From f2b4c0e26965397934afa44f8deaeef55694233a Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 09:03:31 -0700 Subject: [PATCH 18/28] Merge listener signatures into intializer list in PacketListener --- libraries/networking/src/PacketReceiver.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 12f9b618bb..e7fff8e679 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -132,8 +132,10 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* static const QString NON_SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer"; static const QString NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS = "QSharedPointer"; - QSet possibleSignatures { QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKET_LISTENER_PARAMETERS) }; - possibleSignatures << QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS); + QSet possibleSignatures { + QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKET_LISTENER_PARAMETERS), + QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS) + }; if (!NON_SOURCED_PACKETS.contains(type)) { static const QString SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer,QSharedPointer"; From 5ba3280975ab0444b08c4e5de562f92b235325c9 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 10:01:16 -0700 Subject: [PATCH 19/28] Remove include in NLPacketList --- libraries/networking/src/NLPacketList.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/networking/src/NLPacketList.h b/libraries/networking/src/NLPacketList.h index cb48db08f2..4f3ba49d29 100644 --- a/libraries/networking/src/NLPacketList.h +++ b/libraries/networking/src/NLPacketList.h @@ -12,8 +12,6 @@ #ifndef hifi_NLPacketList_h #define hifi_NLPacketList_h -#include - #include "udt/PacketList.h" #include "NLPacket.h" From da7c9198c5bc962f80271518ead95ad6da92c053 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 10:01:40 -0700 Subject: [PATCH 20/28] Remove superfluous log message --- domain-server/src/DomainServerSettingsManager.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index c9b52a17bb..fc0ed95b92 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -68,8 +68,6 @@ DomainServerSettingsManager::DomainServerSettingsManager() : } void DomainServerSettingsManager::processSettingsRequestPacket(QSharedPointer packet) { - qDebug() << "Got request for domain settings"; - Assignment::Type type; packet->readPrimitive(&type); From ebf112e2003469cebc0d96b4a6163f36a5077c48 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 10:03:52 -0700 Subject: [PATCH 21/28] Replace NLPacketList::getSourceID() with const& return --- libraries/networking/src/NLPacketList.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/NLPacketList.h b/libraries/networking/src/NLPacketList.h index 4f3ba49d29..5391e49488 100644 --- a/libraries/networking/src/NLPacketList.h +++ b/libraries/networking/src/NLPacketList.h @@ -21,7 +21,7 @@ public: NLPacketList(PacketType packetType, QByteArray extendedHeader = QByteArray(), bool isReliable = false, bool isOrdered = false); NLPacketList(PacketList&& packetList); - QUuid getSourceID() const { return _sourceID; } + const QUuid& getSourceID() const { return _sourceID; } private: NLPacketList(const NLPacketList& other) = delete; From df510693058afcd0c03a559a30a47acfd0b85603 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 10:39:19 -0700 Subject: [PATCH 22/28] Rename getAllData to getMessage --- libraries/networking/src/DomainHandler.cpp | 2 +- libraries/networking/src/udt/PacketList.cpp | 2 +- libraries/networking/src/udt/PacketList.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index 81b7ee6c55..62b00a8c98 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -283,7 +283,7 @@ void DomainHandler::settingsRequestFinished() { } void DomainHandler::processSettingsPacketList(QSharedPointer packetList) { - auto data = packetList->getAllData(); + auto data = packetList->getMessage(); _settingsObject = QJsonDocument::fromJson(data).object(); diff --git a/libraries/networking/src/udt/PacketList.cpp b/libraries/networking/src/udt/PacketList.cpp index 23060fecf4..2e9bef09e1 100644 --- a/libraries/networking/src/udt/PacketList.cpp +++ b/libraries/networking/src/udt/PacketList.cpp @@ -80,7 +80,7 @@ std::unique_ptr PacketList::createPacketWithExtendedHeader() { return packet; } -QByteArray PacketList::getAllData() { +QByteArray PacketList::getMessage() { size_t sizeBytes = 0; for (const auto& packet : _packets) { diff --git a/libraries/networking/src/udt/PacketList.h b/libraries/networking/src/udt/PacketList.h index 5596253a6c..37c253ac08 100644 --- a/libraries/networking/src/udt/PacketList.h +++ b/libraries/networking/src/udt/PacketList.h @@ -50,7 +50,7 @@ public: void closeCurrentPacket(bool shouldSendEmpty = false); - QByteArray getAllData(); + QByteArray getMessage(); template qint64 readPrimitive(T* data); template qint64 writePrimitive(const T& data); From fc29297d871d2589c5fcd5151e2ccbc58b342dad Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 10:55:26 -0700 Subject: [PATCH 23/28] Update SendQueue::queuePacketList to use splice --- libraries/networking/src/udt/SendQueue.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 650542dec5..0e97e9a1e0 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -68,20 +68,19 @@ void SendQueue::queuePacketList(std::unique_ptr packetList) { Q_ASSERT(packetList->_packets.size() > 0); { - QWriteLocker locker(&_packetsLock); - auto messageNumber = getNextMessageNumber(); if (packetList->_packets.size() == 1) { - auto packet = packetList->takeFront(); - packet->setPacketPosition(Packet::PacketPosition::ONLY); + auto packet = packetList->_packets.front().get(); + packet->setPacketPosition(Packet::PacketPosition::ONLY); packet->writeMessageNumber(messageNumber); - _packets.push_back(std::move(packet)); } else { bool haveMarkedFirstPacket = false; - while (!packetList->_packets.empty()) { - auto packet = packetList->takeFront(); + auto end = packetList->_packets.end(); + for (auto it = packetList->_packets.begin(); it != end; ++it) { + auto packet = it->get(); + if (!haveMarkedFirstPacket) { packet->setPacketPosition(Packet::PacketPosition::FIRST); haveMarkedFirstPacket = true; @@ -92,11 +91,14 @@ void SendQueue::queuePacketList(std::unique_ptr packetList) { } packet->writeMessageNumber(messageNumber); - - _packets.push_back(std::move(packet)); } } + + QWriteLocker locker(&_packetsLock); + + _packets.splice(_packets.end(), packetList->_packets); } + if (!this->thread()->isRunning()) { this->thread()->start(); } From 121d3a77ad6398aa4146d4ba0770965d4ef55a64 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 11:27:31 -0700 Subject: [PATCH 24/28] Fix iterator usage in SendQueue --- libraries/networking/src/udt/SendQueue.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 0e97e9a1e0..cfb969a186 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -71,20 +71,21 @@ void SendQueue::queuePacketList(std::unique_ptr packetList) { auto messageNumber = getNextMessageNumber(); if (packetList->_packets.size() == 1) { - auto packet = packetList->_packets.front().get(); + auto& packet = packetList->_packets.front(); packet->setPacketPosition(Packet::PacketPosition::ONLY); packet->writeMessageNumber(messageNumber); } else { bool haveMarkedFirstPacket = false; auto end = packetList->_packets.end(); + auto lastElement = --packetList->_packets.end(); for (auto it = packetList->_packets.begin(); it != end; ++it) { - auto packet = it->get(); + auto& packet = *it; if (!haveMarkedFirstPacket) { packet->setPacketPosition(Packet::PacketPosition::FIRST); haveMarkedFirstPacket = true; - } else if (packetList->_packets.empty()) { + } else if (it == lastElement) { packet->setPacketPosition(Packet::PacketPosition::LAST); } else { packet->setPacketPosition(Packet::PacketPosition::MIDDLE); From d18ce5066f10d2965e834d726249ff9da7ad576e Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 19 Aug 2015 11:55:06 -0700 Subject: [PATCH 25/28] Fix error with Assignment::Type not being hashable --- libraries/networking/src/Assignment.cpp | 7 +++++++ libraries/networking/src/Assignment.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/libraries/networking/src/Assignment.cpp b/libraries/networking/src/Assignment.cpp index e8ba67c4a6..6a89bd80c4 100644 --- a/libraries/networking/src/Assignment.cpp +++ b/libraries/networking/src/Assignment.cpp @@ -159,3 +159,10 @@ QDataStream& operator>>(QDataStream &in, Assignment& assignment) { return in; } + + +uint qHash(const Assignment::Type& key, uint seed) { + // seems odd that Qt couldn't figure out this cast itself, but this fixes a compile error after switch to + // strongly typed enum for PacketType + return qHash((uint8_t) key, seed); +} diff --git a/libraries/networking/src/Assignment.h b/libraries/networking/src/Assignment.h index 0fadc78770..fc2f8620a2 100644 --- a/libraries/networking/src/Assignment.h +++ b/libraries/networking/src/Assignment.h @@ -100,4 +100,6 @@ protected: QUuid _walletUUID; /// the UUID for the wallet that should be paid for this assignment }; +uint qHash(const Assignment::Type& key, uint seed); + #endif // hifi_Assignment_h From 62c76d0332baedf401eb88d75bb69f2d35c30eab Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 20 Aug 2015 14:55:51 +0200 Subject: [PATCH 26/28] registerListenerForTypes optimisations --- assignment-client/src/audio/AudioMixer.cpp | 11 ++- .../src/octree/OctreePacketProcessor.cpp | 9 +-- libraries/networking/src/PacketReceiver.cpp | 69 +++++++++---------- libraries/networking/src/PacketReceiver.h | 10 ++- 4 files changed, 48 insertions(+), 51 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 664088084d..c1d0cc2215 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -96,14 +96,11 @@ AudioMixer::AudioMixer(NLPacket& packet) : // SOON auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); - - QSet nodeAudioPackets { - PacketType::MicrophoneAudioNoEcho, PacketType::MicrophoneAudioWithEcho, - PacketType::InjectAudio, PacketType::SilentAudioFrame, - PacketType::AudioStreamStats - }; - packetReceiver.registerListenerForTypes(nodeAudioPackets, this, "handleNodeAudioPacket"); + packetReceiver.registerListenerForTypes({ PacketType::MicrophoneAudioNoEcho, PacketType::MicrophoneAudioWithEcho, + PacketType::InjectAudio, PacketType::SilentAudioFrame, + PacketType::AudioStreamStats }, + this, "handleNodeAudioPacket"); packetReceiver.registerListener(PacketType::MuteEnvironment, this, "handleMuteEnvironmentPacket"); } diff --git a/interface/src/octree/OctreePacketProcessor.cpp b/interface/src/octree/OctreePacketProcessor.cpp index 9bf845cccd..5b8ff78fad 100644 --- a/interface/src/octree/OctreePacketProcessor.cpp +++ b/interface/src/octree/OctreePacketProcessor.cpp @@ -18,13 +18,10 @@ OctreePacketProcessor::OctreePacketProcessor() { auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); - - QSet types { - PacketType::OctreeStats, PacketType::EntityData, - PacketType::EntityErase, PacketType::OctreeStats - }; - packetReceiver.registerDirectListenerForTypes(types, this, "handleOctreePacket"); + packetReceiver.registerDirectListenerForTypes({ PacketType::OctreeStats, PacketType::EntityData, + PacketType::EntityErase, PacketType::OctreeStats }, + this, "handleOctreePacket"); } void OctreePacketProcessor::handleOctreePacket(QSharedPointer packet, SharedNodePointer senderNode) { diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index e7fff8e679..7a9a3b1ddd 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -19,49 +19,48 @@ #include "NodeList.h" #include "SharedUtil.h" -PacketReceiver::PacketReceiver(QObject* parent) : - QObject(parent), - _packetListenerMap() -{ +PacketReceiver::PacketReceiver(QObject* parent) : QObject(parent) { qRegisterMetaType>(); } -bool PacketReceiver::registerListenerForTypes(const QSet& types, QObject* listener, const char* slot) { - QSet nonSourcedTypes; - QSet sourcedTypes; - - foreach(PacketType type, types) { - if (NON_SOURCED_PACKETS.contains(type)) { - nonSourcedTypes << type; - } else { - sourcedTypes << type; - } - } - - Q_ASSERT(listener); - - if (nonSourcedTypes.size() > 0) { - QMetaMethod nonSourcedMethod = matchingMethodForListener(*nonSourcedTypes.begin(), listener, slot); - if (nonSourcedMethod.isValid()) { - foreach(PacketType type, nonSourcedTypes) { - registerVerifiedListener(type, listener, nonSourcedMethod); - } - } else { +bool PacketReceiver::registerListenerForTypes(PacketTypeList types, QObject* listener, const char* slot) { + Q_ASSERT_X(!types.empty(), "PacketReceiver::registerListenerForTypes", "No types to register"); + Q_ASSERT_X(listener, "PacketReceiver::registerListenerForTypes", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerListenerForTypes", "No slot to register"); + + // Partition types based on whether they are sourced or not (non sourced in front) + auto middle = std::partition(std::begin(types), std::end(types), [](PacketType type) { + return NON_SOURCED_PACKETS.contains(type); + }); + + QMetaMethod nonSourcedMethod, sourcedMethod; + + // Check we have a valid method for non sourced types if any + if (middle != std::begin(types)) { + nonSourcedMethod = matchingMethodForListener(*std::begin(types), listener, slot); + if (!nonSourcedMethod.isValid()) { return false; } } - - if (sourcedTypes.size() > 0) { - QMetaMethod sourcedMethod = matchingMethodForListener(*sourcedTypes.begin(), listener, slot); - if (sourcedMethod.isValid()) { - foreach(PacketType type, sourcedTypes) { - registerVerifiedListener(type, listener, sourcedMethod); - } - } else { + + // Check we have a valid method for sourced types if any + if (middle != std::end(types)) { + sourcedMethod = matchingMethodForListener(*middle, listener, slot); + if (!sourcedMethod.isValid()) { return false; } } + // Register non sourced types + std::for_each(std::begin(types), middle, [this, &listener, &nonSourcedMethod](PacketType type) { + registerVerifiedListener(type, listener, nonSourcedMethod); + }); + + // Register sourced types + std::for_each(middle, std::end(types), [this, &listener, &sourcedMethod](PacketType type) { + registerVerifiedListener(type, listener, sourcedMethod); + }); + return true; } @@ -77,10 +76,10 @@ void PacketReceiver::registerDirectListener(PacketType type, QObject* listener, } } -void PacketReceiver::registerDirectListenerForTypes(const QSet& types, +void PacketReceiver::registerDirectListenerForTypes(PacketTypeList types, QObject* listener, const char* slot) { // just call register listener for types to start - bool success = registerListenerForTypes(types, listener, slot); + bool success = registerListenerForTypes(std::move(types), listener, slot); if (success) { _directConnectSetMutex.lock(); diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index b5a4501476..9965eccdc2 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -13,6 +13,8 @@ #ifndef hifi_PacketReceiver_h #define hifi_PacketReceiver_h +#include + #include #include #include @@ -30,6 +32,8 @@ class OctreePacketProcessor; class PacketReceiver : public QObject { Q_OBJECT public: + using PacketTypeList = std::vector; + PacketReceiver(QObject* parent = 0); PacketReceiver(const PacketReceiver&) = delete; @@ -42,8 +46,8 @@ public: void resetCounters() { _inPacketCount = 0; _inByteCount = 0; } - bool registerListenerForTypes(const QSet& types, QObject* listener, const char* slot); - bool registerMessageListener(PacketType types, QObject* listener, const char* slot); + bool registerListenerForTypes(PacketTypeList types, QObject* listener, const char* slot); + bool registerMessageListener(PacketType type, QObject* listener, const char* slot); bool registerListener(PacketType type, QObject* listener, const char* slot); void unregisterListener(QObject* listener); @@ -56,7 +60,7 @@ signals: private: // these are brutal hacks for now - ideally GenericThread / ReceivedPacketProcessor // should be changed to have a true event loop and be able to handle our QMetaMethod::invoke - void registerDirectListenerForTypes(const QSet& types, QObject* listener, const char* slot); + void registerDirectListenerForTypes(PacketTypeList types, QObject* listener, const char* slot); void registerDirectListener(PacketType type, QObject* listener, const char* slot); QMetaMethod matchingMethodForListener(PacketType type, QObject* object, const char* slot) const; From f6854782a4ce3beee478170362e69a1835558c93 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 20 Aug 2015 14:57:35 +0200 Subject: [PATCH 27/28] Debug/Lock cleanup --- libraries/networking/src/PacketReceiver.cpp | 92 ++++++++++----------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 7a9a3b1ddd..a086949ac8 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -65,45 +65,49 @@ bool PacketReceiver::registerListenerForTypes(PacketTypeList types, QObject* lis } void PacketReceiver::registerDirectListener(PacketType type, QObject* listener, const char* slot) { + Q_ASSERT_X(listener, "PacketReceiver::registerDirectListener", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerDirectListener", "No slot to register"); + bool success = registerListener(type, listener, slot); if (success) { - _directConnectSetMutex.lock(); + QMutexLocker locker(&_directConnectSetMutex); // if we successfully registered, add this object to the set of objects that are directly connected _directlyConnectedObjects.insert(listener); - - _directConnectSetMutex.unlock(); } } void PacketReceiver::registerDirectListenerForTypes(PacketTypeList types, QObject* listener, const char* slot) { + Q_ASSERT_X(listener, "PacketReceiver::registerDirectListenerForTypes", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerDirectListenerForTypes", "No slot to register"); + // just call register listener for types to start bool success = registerListenerForTypes(std::move(types), listener, slot); if (success) { - _directConnectSetMutex.lock(); + QMutexLocker locker(&_directConnectSetMutex); // if we successfully registered, add this object to the set of objects that are directly connected _directlyConnectedObjects.insert(listener); - - _directConnectSetMutex.unlock(); } } bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, const char* slot) { + Q_ASSERT_X(listener, "PacketReceiver::registerMessageListener", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerMessageListener", "No slot to register"); + QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); if (matchingMethod.isValid()) { QMutexLocker locker(&_packetListenerLock); if (_packetListListenerMap.contains(type)) { - qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type + qCWarning(networking) << "Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } // add the mapping _packetListListenerMap[type] = ObjectMethodPair(QPointer(listener), matchingMethod); - return true; } else { return false; @@ -111,7 +115,8 @@ bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, } bool PacketReceiver::registerListener(PacketType type, QObject* listener, const char* slot) { - Q_ASSERT(listener); + Q_ASSERT_X(listener, "PacketReceiver::registerListener", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerListener", "No slot to register"); QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); @@ -124,16 +129,18 @@ bool PacketReceiver::registerListener(PacketType type, QObject* listener, const } QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* object, const char* slot) const { - Q_ASSERT(object); + Q_ASSERT_X(object, "PacketReceiver::matchingMethodForListener", "No object to call"); + Q_ASSERT_X(slot, "PacketReceiver::matchingMethodForListener", "No slot to call"); // normalize the slot with the expected parameters - + + static const QString SIGNATURE_TEMPLATE("%1(%2)"); static const QString NON_SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer"; static const QString NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS = "QSharedPointer"; QSet possibleSignatures { - QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKET_LISTENER_PARAMETERS), - QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS) + SIGNATURE_TEMPLATE.arg(slot, NON_SOURCED_PACKET_LISTENER_PARAMETERS), + SIGNATURE_TEMPLATE.arg(slot, NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS) }; if (!NON_SOURCED_PACKETS.contains(type)) { @@ -145,10 +152,10 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* // a sourced packet must take the shared pointer to the packet but optionally could include // a shared pointer to the node - possibleSignatures << QString("%1(%2)").arg(slot).arg(TYPEDEF_SOURCED_PACKET_LISTENER_PARAMETERS); - possibleSignatures << QString("%1(%2)").arg(slot).arg(SOURCED_PACKET_LISTENER_PARAMETERS); - possibleSignatures << QString("%1(%2)").arg(slot).arg(TYPEDEF_SOURCED_PACKETLIST_LISTENER_PARAMETERS); - possibleSignatures << QString("%1(%2)").arg(slot).arg(SOURCED_PACKETLIST_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, TYPEDEF_SOURCED_PACKET_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, SOURCED_PACKET_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, TYPEDEF_SOURCED_PACKETLIST_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, SOURCED_PACKETLIST_LISTENER_PARAMETERS); } int methodIndex = -1; @@ -184,39 +191,30 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* } void PacketReceiver::registerVerifiedListener(PacketType type, QObject* object, const QMetaMethod& slot) { - _packetListenerLock.lock(); + Q_ASSERT_X(object, "PacketReceiver::registerVerifiedListener", "No object to register"); + QMutexLocker locker(&_packetListenerLock); if (_packetListenerMap.contains(type)) { - qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type + qCWarning(networking) << "Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } // add the mapping _packetListenerMap[type] = ObjectMethodPair(QPointer(object), slot); - - _packetListenerLock.unlock(); - } void PacketReceiver::unregisterListener(QObject* listener) { - _packetListenerLock.lock(); - - auto it = _packetListenerMap.begin(); - - while (it != _packetListenerMap.end()) { - if (it.value().first == listener) { - // this listener matches - erase it - it = _packetListenerMap.erase(it); - } else { - ++it; - } - } - - _packetListenerLock.unlock(); + Q_ASSERT_X(listener, "PacketReceiver::unregisterListener", "No listener to unregister"); - _directConnectSetMutex.lock(); + QMutexLocker packetListenerLocker(&_packetListenerLock); + std::remove_if(std::begin(_packetListenerMap), std::end(_packetListenerMap), + [&listener](const ObjectMethodPair& pair) { + return pair.first == listener; + }); + packetListenerLocker.unlock(); + + QMutexLocker directConnectSetLocker(&_directConnectSetMutex); _directlyConnectedObjects.remove(listener); - _directConnectSetMutex.unlock(); } void PacketReceiver::handleVerifiedPacketList(std::unique_ptr packetList) { @@ -337,7 +335,7 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p } } else if (it == _packetListListenerMap.end()) { - qWarning() << "No listener found for packet type" << nlPacketList->getType(); + qCWarning(networking) << "No listener found for packet type" << nlPacketList->getType(); // insert a dummy listener so we don't print this again _packetListListenerMap.insert(nlPacketList->getType(), { nullptr, QMetaMethod() }); @@ -371,7 +369,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } } - _packetListenerLock.lock(); + QMutexLocker packetListenerLocker(&_packetListenerLock); bool listenerIsDead = false; @@ -386,12 +384,10 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { bool success = false; // check if this is a directly connected listener - _directConnectSetMutex.lock(); - + QMutexLocker directConnectSetLocker(&_directConnectSetMutex); Qt::ConnectionType connectionType = _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; - - _directConnectSetMutex.unlock(); + directConnectSetLocker.unlock(); PacketType packetType = nlPacket->getType(); @@ -456,18 +452,18 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { it = _packetListenerMap.erase(it); // if it exists, remove the listener from _directlyConnectedObjects - _directConnectSetMutex.lock(); + QMutexLocker locker(&_directConnectSetMutex); _directlyConnectedObjects.remove(listener.first); - _directConnectSetMutex.unlock(); + locker.unlock(); } } else if (it == _packetListenerMap.end()) { - qWarning() << "No listener found for packet type" << nlPacket->getType(); + qCWarning(networking) << "No listener found for packet type" << nlPacket->getType(); // insert a dummy listener so we don't print this again _packetListenerMap.insert(nlPacket->getType(), { nullptr, QMetaMethod() }); } - _packetListenerLock.unlock(); + packetListenerLocker.unlock(); } From 74d6e5ba890186c904a767974f09f0e98bcef938 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 20 Aug 2015 10:01:12 -0700 Subject: [PATCH 28/28] make sure LNL goes down before domain-server --- domain-server/src/DomainServer.cpp | 5 +++++ domain-server/src/DomainServer.h | 3 ++- libraries/networking/src/udt/Connection.cpp | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index ddbef90ef6..410b58d17f 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -110,6 +110,11 @@ DomainServer::DomainServer(int argc, char* argv[]) : } } +DomainServer::~DomainServer() { + // destroy the LimitedNodeList before the DomainServer QCoreApplication is down + DependencyManager::destroy(); +} + void DomainServer::aboutToQuit() { // clear the log handler so that Qt doesn't call the destructor on LogHandler diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 7495e080de..c39633f62a 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -38,7 +38,8 @@ class DomainServer : public QCoreApplication, public HTTPSRequestHandler { Q_OBJECT public: DomainServer(int argc, char* argv[]); - + ~DomainServer(); + static int const EXIT_CODE_REBOOT; bool handleHTTPRequest(HTTPConnection* connection, const QUrl& url, bool skipSubHandler = false); diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 85c3dbcbda..9f8b1eb3ee 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -58,7 +58,8 @@ Connection::~Connection() { _sendQueue->deleteLater(); _sendQueue.release(); - // wait on the send queue thread so we know the send queue is gone + // wait on the send queue thread so we know the send queue is gone + sendQueueThread->quit(); sendQueueThread->wait(); } }