diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index e40f453238..b114492780 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -221,18 +221,18 @@ bool LimitedNodeList::packetVersionAndHashMatch(const QByteArray& packet) { // qint64 LimitedNodeList::readDatagram(char* data, qint64 maxSize, QHostAddress* address = 0, quint16 * port = 0) { -qint64 LimitedNodeList::readDatagram(QByteArray& incomingPacket, QHostAddress* address = 0, quint16 * port = 0) { - qint64 result = getNodeSocket().readDatagram(incomingPacket.data(), incomingPacket.size(), address, port); - - SharedNodePointer sendingNode = sendingNodeForPacket(incomingPacket); - if (sendingNode) { - emit dataReceived(sendingNode->getType(), incomingPacket.size()); - } else { - emit dataReceived(NodeType::Unassigned, incomingPacket.size()); - } - - return result; -} +//qint64 LimitedNodeList::readDatagram(QByteArray& incomingPacket, QHostAddress* address = 0, quint16* port = 0) { +// qint64 result = getNodeSocket().readDatagram(incomingPacket.data(), incomingPacket.size(), address, port); +// +// SharedNodePointer sendingNode = sendingNodeForPacket(incomingPacket); +// if (sendingNode) { +// emit dataReceived(sendingNode->getType(), incomingPacket.size()); +// } else { +// emit dataReceived(NodeType::Unassigned, incomingPacket.size()); +// } +// +// return result; +//} qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr) { // XXX can BandwidthRecorder be used for this? @@ -250,106 +250,106 @@ qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSock return bytesWritten; } -qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, - const SharedNodePointer& destinationNode, - const HifiSockAddr& overridenSockAddr) { - if (destinationNode) { - PacketType::Value packetType = packetTypeForPacket(datagram); - - if (NON_VERIFIED_PACKETS.contains(packetType)) { - return writeUnverifiedDatagram(datagram, destinationNode, overridenSockAddr); - } - - // if we don't have an overridden address, assume they want to send to the node's active socket - const HifiSockAddr* destinationSockAddr = &overridenSockAddr; - if (overridenSockAddr.isNull()) { - if (destinationNode->getActiveSocket()) { - // use the node's active socket as the destination socket - destinationSockAddr = destinationNode->getActiveSocket(); - } else { - // we don't have a socket to send to, return 0 - return 0; - } - } - - QByteArray datagramCopy = datagram; - - // if we're here and the connection secret is null, debug out - this could be a problem - if (destinationNode->getConnectionSecret().isNull()) { - qDebug() << "LimitedNodeList::writeDatagram called for verified datagram with null connection secret for" - << "destination node" << destinationNode->getUUID() << " - this is either not secure or will cause" - << "this packet to be unverifiable on the receiving side."; - } - - // perform replacement of hash and optionally also sequence number in the header - if (SEQUENCE_NUMBERED_PACKETS.contains(packetType)) { - PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode->getUUID(), packetType); - replaceHashAndSequenceNumberInPacket(datagramCopy, destinationNode->getConnectionSecret(), - sequenceNumber, packetType); - } else { - replaceHashInPacket(datagramCopy, destinationNode->getConnectionSecret(), packetType); - } - - emit dataSent(destinationNode->getType(), datagram.size()); - auto bytesWritten = writeDatagram(datagramCopy, *destinationSockAddr); - // Keep track of per-destination-node bandwidth - destinationNode->recordBytesSent(bytesWritten); - return bytesWritten; - } - - // didn't have a destinationNode to send to, return 0 - return 0; -} - -qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, const SharedNodePointer& destinationNode, - const HifiSockAddr& overridenSockAddr) { - if (destinationNode) { - // if we don't have an ovveriden address, assume they want to send to the node's active socket - const HifiSockAddr* destinationSockAddr = &overridenSockAddr; - if (overridenSockAddr.isNull()) { - if (destinationNode->getActiveSocket()) { - // use the node's active socket as the destination socket - destinationSockAddr = destinationNode->getActiveSocket(); - } else { - // we don't have a socket to send to, return 0 - return 0; - } - } - - PacketType::Value packetType = packetTypeForPacket(datagram); - - // optionally peform sequence number replacement in the header - if (SEQUENCE_NUMBERED_PACKETS.contains(packetType)) { - - QByteArray datagramCopy = datagram; - - PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode->getUUID(), packetType); - replaceSequenceNumberInPacket(datagramCopy, sequenceNumber, packetType); - - // send the datagram with sequence number replaced in header - return writeDatagram(datagramCopy, *destinationSockAddr); - } else { - return writeDatagram(datagram, *destinationSockAddr); - } - } - - // didn't have a destinationNode to send to, return 0 - return 0; -} - -qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr) { - return writeDatagram(datagram, destinationSockAddr); -} - -qint64 LimitedNodeList::writeDatagram(const char* data, qint64 size, const SharedNodePointer& destinationNode, - const HifiSockAddr& overridenSockAddr) { - return writeDatagram(QByteArray(data, size), destinationNode, overridenSockAddr); -} - -qint64 LimitedNodeList::writeUnverifiedDatagram(const char* data, qint64 size, const SharedNodePointer& destinationNode, - const HifiSockAddr& overridenSockAddr) { - return writeUnverifiedDatagram(QByteArray(data, size), destinationNode, overridenSockAddr); -} +//qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, +// const SharedNodePointer& destinationNode, +// const HifiSockAddr& overridenSockAddr) { +// if (destinationNode) { +// PacketType::Value packetType = packetTypeForPacket(datagram); +// +// if (NON_VERIFIED_PACKETS.contains(packetType)) { +// return writeUnverifiedDatagram(datagram, destinationNode, overridenSockAddr); +// } +// +// // if we don't have an overridden address, assume they want to send to the node's active socket +// const HifiSockAddr* destinationSockAddr = &overridenSockAddr; +// if (overridenSockAddr.isNull()) { +// if (destinationNode->getActiveSocket()) { +// // use the node's active socket as the destination socket +// destinationSockAddr = destinationNode->getActiveSocket(); +// } else { +// // we don't have a socket to send to, return 0 +// return 0; +// } +// } +// +// QByteArray datagramCopy = datagram; +// +// // if we're here and the connection secret is null, debug out - this could be a problem +// if (destinationNode->getConnectionSecret().isNull()) { +// qDebug() << "LimitedNodeList::writeDatagram called for verified datagram with null connection secret for" +// << "destination node" << destinationNode->getUUID() << " - this is either not secure or will cause" +// << "this packet to be unverifiable on the receiving side."; +// } +// +// // perform replacement of hash and optionally also sequence number in the header +// if (SEQUENCE_NUMBERED_PACKETS.contains(packetType)) { +// PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode->getUUID(), packetType); +// replaceHashAndSequenceNumberInPacket(datagramCopy, destinationNode->getConnectionSecret(), +// sequenceNumber, packetType); +// } else { +// replaceHashInPacket(datagramCopy, destinationNode->getConnectionSecret(), packetType); +// } +// +// emit dataSent(destinationNode->getType(), datagram.size()); +// auto bytesWritten = writeDatagram(datagramCopy, *destinationSockAddr); +// // Keep track of per-destination-node bandwidth +// destinationNode->recordBytesSent(bytesWritten); +// return bytesWritten; +// } +// +// // didn't have a destinationNode to send to, return 0 +// return 0; +//} +// +//qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, const SharedNodePointer& destinationNode, +// const HifiSockAddr& overridenSockAddr) { +// if (destinationNode) { +// // if we don't have an ovveriden address, assume they want to send to the node's active socket +// const HifiSockAddr* destinationSockAddr = &overridenSockAddr; +// if (overridenSockAddr.isNull()) { +// if (destinationNode->getActiveSocket()) { +// // use the node's active socket as the destination socket +// destinationSockAddr = destinationNode->getActiveSocket(); +// } else { +// // we don't have a socket to send to, return 0 +// return 0; +// } +// } +// +// PacketType::Value packetType = packetTypeForPacket(datagram); +// +// // optionally peform sequence number replacement in the header +// if (SEQUENCE_NUMBERED_PACKETS.contains(packetType)) { +// +// QByteArray datagramCopy = datagram; +// +// PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode->getUUID(), packetType); +// replaceSequenceNumberInPacket(datagramCopy, sequenceNumber, packetType); +// +// // send the datagram with sequence number replaced in header +// return writeDatagram(datagramCopy, *destinationSockAddr); +// } else { +// return writeDatagram(datagram, *destinationSockAddr); +// } +// } +// +// // didn't have a destinationNode to send to, return 0 +// return 0; +//} +// +//qint64 LimitedNodeList::writeUnverifiedDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr) { +// return writeDatagram(datagram, destinationSockAddr); +//} +// +//qint64 LimitedNodeList::writeDatagram(const char* data, qint64 size, const SharedNodePointer& destinationNode, +// const HifiSockAddr& overridenSockAddr) { +// return writeDatagram(QByteArray(data, size), destinationNode, overridenSockAddr); +//} +// +//qint64 LimitedNodeList::writeUnverifiedDatagram(const char* data, qint64 size, const SharedNodePointer& destinationNode, +// const HifiSockAddr& overridenSockAddr) { +// return writeUnverifiedDatagram(QByteArray(data, size), destinationNode, overridenSockAddr); +//} PacketSequenceNumber LimitedNodeList::getNextSequenceNumberForPacket(const QUuid& nodeUUID, PacketType::Value packetType) { // Thanks to std::map and std::unordered_map this line either default constructs the diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index 524983a8ff..4f12b761d1 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -37,9 +37,17 @@ std::unique_ptr NLPacket::create(PacketType::Value type, int64_t size) return std::unique_ptr(new NLPacket(type, size)); } +std::unique_ptr NLPacket::createCopy(const std::unique_ptr& other) { + Q_ASSERT(other); + return std::unique_ptr(new NLPacket(*other)); +} + NLPacket::NLPacket(PacketType::Value type, int64_t size) : Packet(type, localHeaderSize(type) + size) { } +NLPacket::NLPacket(NLPacket& other) : Packet(other) { +} + void NLPacket::setSourceUuid(QUuid sourceUuid) { Q_ASSERT(!NON_SOURCED_PACKETS.contains(_type)); auto offset = Packet::totalHeadersSize(); diff --git a/libraries/networking/src/NLPacket.h b/libraries/networking/src/NLPacket.h index 7f625fb60c..9105418492 100644 --- a/libraries/networking/src/NLPacket.h +++ b/libraries/networking/src/NLPacket.h @@ -17,6 +17,8 @@ class NLPacket : public Packet { public: static std::unique_ptr create(PacketType::Value type, int64_t size = -1); + // Provided for convenience, try to limit use + static std::unique_ptr createCopy(const std::unique_ptr& other); static int64_t localHeaderSize(PacketType::Value type); static int64_t maxPayloadSize(PacketType::Value type); @@ -26,6 +28,7 @@ public: protected: NLPacket(PacketType::Value type, int64_t size); + NLPacket(NLPacket& other); void setSourceUuid(QUuid sourceUuid); void setConnectionUuid(QUuid connectionUuid); diff --git a/libraries/networking/src/Packet.cpp b/libraries/networking/src/Packet.cpp index 65d0899a94..b11564428f 100644 --- a/libraries/networking/src/Packet.cpp +++ b/libraries/networking/src/Packet.cpp @@ -38,7 +38,6 @@ std::unique_ptr Packet::create(PacketType::Value type, qint64 size) { return std::unique_ptr(new Packet(type, size)); } - std::unique_ptr Packet::createCopy(const std::unique_ptr& other) { Q_ASSERT(other); return std::unique_ptr(new Packet(*other)); diff --git a/libraries/networking/src/Packet.h b/libraries/networking/src/Packet.h index 9d58d1e2e1..471237b212 100644 --- a/libraries/networking/src/Packet.h +++ b/libraries/networking/src/Packet.h @@ -61,6 +61,10 @@ public: protected: Packet(PacketType::Value type, int64_t size); + Packet(const Packet& other); + Packet& operator=(const Packet& other); + Packet(Packet&& other); + Packet& operator=(Packet&& other); // QIODevice virtual functions virtual qint64 writeData(const char* data, qint64 maxSize); @@ -79,12 +83,6 @@ protected: qint64 _capacity = 0; // Total capacity of the payload qint64 _sizeUsed = 0; // How much of the payload is actually used - -private: - Packet(const Packet& other); - Packet& operator=(const Packet& other); - Packet(Packet&& other); - Packet& operator=(Packet&& other); }; #endif // hifi_Packet_h \ No newline at end of file diff --git a/libraries/networking/src/PacketHeaders.cpp b/libraries/networking/src/PacketHeaders.cpp index cab062165f..51d0babc39 100644 --- a/libraries/networking/src/PacketHeaders.cpp +++ b/libraries/networking/src/PacketHeaders.cpp @@ -23,9 +23,9 @@ const QSet NON_VERIFIED_PACKETS = QSet() << CreateAssignment << RequestAssignment << StunResponse << NodeJsonStats << EntityQuery << OctreeDataNack << EntityEditNack - << IceServerHeartbeat << IceServerPeerInformation - << IceServerQuery << UnverifiedPing - << UnverifiedPingReply << StopNode + << ICEServerHeartbeat << ICEServerPeerInformation + << ICEServerQuery << Ping + << PingReply << StopNode << DomainServerPathQuery << DomainServerPathResponse << DomainServerAddedNode; @@ -103,8 +103,8 @@ PacketVersion versionForPacketType(PacketType::Value packetType) { return 2; case AudioStreamStats: return 1; - case IceServerHeartbeat: - case IceServerQuery: + case ICEServerHeartbeat: + case ICEServerQuery: return 1; default: return 0; @@ -152,12 +152,12 @@ QString nameForPacketType(PacketType::Value packetType) { PACKET_TYPE_NAME_LOOKUP(AudioEnvironment); PACKET_TYPE_NAME_LOOKUP(EntityEditNack); PACKET_TYPE_NAME_LOOKUP(SignedTransactionPayment); - PACKET_TYPE_NAME_LOOKUP(IceServerHeartbeat); + PACKET_TYPE_NAME_LOOKUP(ICEServerHeartbeat); PACKET_TYPE_NAME_LOOKUP(DomainServerAddedNode); - PACKET_TYPE_NAME_LOOKUP(IceServerQuery); - PACKET_TYPE_NAME_LOOKUP(IceServerPeerInformation); - PACKET_TYPE_NAME_LOOKUP(UnverifiedPing); - PACKET_TYPE_NAME_LOOKUP(UnverifiedPingReply); + PACKET_TYPE_NAME_LOOKUP(ICEServerQuery); + PACKET_TYPE_NAME_LOOKUP(ICEServerPeerInformation); + PACKET_TYPE_NAME_LOOKUP(ICEPing); + PACKET_TYPE_NAME_LOOKUP(ICEPingReply); PACKET_TYPE_NAME_LOOKUP(EntityAdd); PACKET_TYPE_NAME_LOOKUP(EntityEdit); default: diff --git a/libraries/networking/src/PacketList.cpp b/libraries/networking/src/PacketList.cpp index 01d4c61089..b5b5284dde 100644 --- a/libraries/networking/src/PacketList.cpp +++ b/libraries/networking/src/PacketList.cpp @@ -11,13 +11,17 @@ #include "PacketList.h" -PacketList::PacketList(PacketType::Value packetType) : +#include + +#include "NLPacket.h" + +template PacketList::PacketList(PacketType::Value packetType) : _packetType(packetType) { } -void PacketList::createPacketWithExtendedHeader() { +template std::unique_ptr PacketList::createPacketWithExtendedHeader() { // use the static create method to create a new packet auto packet = T::create(_packetType); @@ -28,7 +32,7 @@ void PacketList::createPacketWithExtendedHeader() { } } -qint64 writeData(const char* data, qint64 maxSize) { +template qint64 PacketList::writeData(const char* data, qint64 maxSize) { if (!_currentPacket) { // we don't have a current packet, time to set one up _currentPacket = createPacketWithExtendedHeader(); @@ -63,13 +67,13 @@ qint64 writeData(const char* data, qint64 maxSize) { } // copy from currentPacket where the segment started to the beginning of the newPacket - newPacket.write(currentPacket->getPayload() + _segmentStartIndex, numBytesToEnd); + newPacket.write(_currentPacket->getPayload() + _segmentStartIndex, numBytesToEnd); // the current segment now starts at the beginning of the new packet _segmentStartIndex = 0; // shrink the current payload to the actual size of the packet - currentPacket.setSizeUsed(_segmentStartIndex); + _currentPacket.setSizeUsed(_segmentStartIndex); } // move the current packet to our list of packets @@ -99,7 +103,7 @@ qint64 writeData(const char* data, qint64 maxSize) { } } -void PacketList::closeCurrentPacket() { +template void PacketList::closeCurrentPacket() { // move the current packet to our list of packets _packets.insert(std::move(_currentPacket)); } diff --git a/libraries/networking/src/PacketList.h b/libraries/networking/src/PacketList.h index e03e152532..a194121324 100644 --- a/libraries/networking/src/PacketList.h +++ b/libraries/networking/src/PacketList.h @@ -16,6 +16,8 @@ #include "PacketHeaders.h" +class NLPacket; + template class PacketList : public QIODevice { public: PacketList(PacketType::Value packetType); @@ -37,7 +39,7 @@ private: std::unique_ptr createPacketWithExtendedHeader(); PacketType::Value _packetType; - bool isOrdered = false; + bool _isOrdered = false; std::unique_ptr _currentPacket; std::list> _packets; diff --git a/libraries/networking/src/SentPacketHistory.cpp b/libraries/networking/src/SentPacketHistory.cpp index 980646a082..2a90f0f094 100644 --- a/libraries/networking/src/SentPacketHistory.cpp +++ b/libraries/networking/src/SentPacketHistory.cpp @@ -8,10 +8,14 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#include -#include "NetworkLogging.h" #include "SentPacketHistory.h" -#include + +#include + +#include + +#include "NetworkLogging.h" +#include "NLPacket.h" SentPacketHistory::SentPacketHistory(int size) : _sentPackets(size), @@ -30,10 +34,13 @@ void SentPacketHistory::packetSent(uint16_t sequenceNumber, const NLPacket& pack << "Expected:" << expectedSequenceNumber << "Actual:" << sequenceNumber; } _newestSequenceNumber = sequenceNumber; - _sentPackets.insert(NLPacket::createCopy(packet)); + + auto temp = std::unique_ptr(const_cast(&packet)); + _sentPackets.insert(NLPacket::createCopy(temp)); + temp.release(); } -const QByteArray* SentPacketHistory::getPacket(uint16_t sequenceNumber) const { +const std::unique_ptr& SentPacketHistory::getPacket(uint16_t sequenceNumber) const { const int UINT16_RANGE = std::numeric_limits::max() + 1; @@ -44,5 +51,5 @@ const QByteArray* SentPacketHistory::getPacket(uint16_t sequenceNumber) const { seqDiff += UINT16_RANGE; } - return _sentPackets.get(seqDiff); + return *_sentPackets.get(seqDiff); } diff --git a/libraries/networking/src/SentPacketHistory.h b/libraries/networking/src/SentPacketHistory.h index 04cfdb3d36..08aa2f9764 100644 --- a/libraries/networking/src/SentPacketHistory.h +++ b/libraries/networking/src/SentPacketHistory.h @@ -17,6 +17,8 @@ #include "SequenceNumberStats.h" +class NLPacket; + class SentPacketHistory { public: diff --git a/libraries/shared/src/RingBufferHistory.h b/libraries/shared/src/RingBufferHistory.h index 9534b2f1db..528d451d76 100644 --- a/libraries/shared/src/RingBufferHistory.h +++ b/libraries/shared/src/RingBufferHistory.h @@ -53,6 +53,18 @@ public: _numEntries++; } } + + // std::unique_ptr need to be passed as an rvalue ref and moved into the vector + void insert(T&& entry) { + // increment newest entry index cyclically + _newestEntryAtIndex = (_newestEntryAtIndex == _size - 1) ? 0 : _newestEntryAtIndex + 1; + + // insert new entry + _buffer[_newestEntryAtIndex] = std::move(entry); + if (_numEntries < _capacity) { + _numEntries++; + } + } // 0 retrieves the most recent entry, _numEntries - 1 retrieves the oldest. // returns NULL if entryAge not within [0, _numEntries-1] @@ -88,7 +100,7 @@ private: int _capacity; int _newestEntryAtIndex; int _numEntries; - QVector _buffer; + std::vector _buffer; public: class Iterator : public std::iterator < std::random_access_iterator_tag, T > {