From 85ebad979f08cdb2ccb8fdaa9a1bb3d0f3f3689e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 28 Jan 2014 11:15:38 -0800 Subject: [PATCH] repair assignment flow errors from packet changes --- assignment-client/src/AssignmentClient.cpp | 56 +++++++++++---------- assignment-client/src/AssignmentFactory.cpp | 8 +-- domain-server/src/DomainServer.cpp | 1 + libraries/shared/src/Assignment.cpp | 9 ++-- libraries/shared/src/PacketHeaders.cpp | 18 ++++--- 5 files changed, 54 insertions(+), 38 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 998ff46217..8725c6ca56 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -124,32 +124,36 @@ void AssignmentClient::readPendingDatagrams() { // construct the deployed assignment from the packet data _currentAssignment = AssignmentFactory::unpackAssignment(receivedPacket); - qDebug() << "Received an assignment -" << *_currentAssignment; - - // switch our nodelist domain IP and port to whoever sent us the assignment - - nodeList->setDomainSockAddr(senderSockAddr); - nodeList->setOwnerUUID(_currentAssignment->getUUID()); - - qDebug() << "Destination IP for assignment is" << nodeList->getDomainIP().toString(); - - // start the deployed assignment - QThread* workerThread = new QThread(this); - - connect(workerThread, SIGNAL(started()), _currentAssignment, SLOT(run())); - - connect(_currentAssignment, SIGNAL(finished()), this, SLOT(assignmentCompleted())); - connect(_currentAssignment, SIGNAL(finished()), workerThread, SLOT(quit())); - connect(_currentAssignment, SIGNAL(finished()), _currentAssignment, SLOT(deleteLater())); - connect(workerThread, SIGNAL(finished()), workerThread, SLOT(deleteLater())); - - _currentAssignment->moveToThread(workerThread); - - // move the NodeList to the thread used for the _current assignment - nodeList->moveToThread(workerThread); - - // Starts an event loop, and emits workerThread->started() - workerThread->start(); + if (_currentAssignment) { + qDebug() << "Received an assignment -" << *_currentAssignment; + + // switch our nodelist domain IP and port to whoever sent us the assignment + + nodeList->setDomainSockAddr(senderSockAddr); + nodeList->setOwnerUUID(_currentAssignment->getUUID()); + + qDebug() << "Destination IP for assignment is" << nodeList->getDomainIP().toString(); + + // start the deployed assignment + QThread* workerThread = new QThread(this); + + connect(workerThread, SIGNAL(started()), _currentAssignment, SLOT(run())); + + connect(_currentAssignment, SIGNAL(finished()), this, SLOT(assignmentCompleted())); + connect(_currentAssignment, SIGNAL(finished()), workerThread, SLOT(quit())); + connect(_currentAssignment, SIGNAL(finished()), _currentAssignment, SLOT(deleteLater())); + connect(workerThread, SIGNAL(finished()), workerThread, SLOT(deleteLater())); + + _currentAssignment->moveToThread(workerThread); + + // move the NodeList to the thread used for the _current assignment + nodeList->moveToThread(workerThread); + + // Starts an event loop, and emits workerThread->started() + workerThread->start(); + } else { + qDebug() << "Received an assignment that could not be unpacked. Re-requesting."; + } } } else { // have the NodeList attempt to handle it diff --git a/assignment-client/src/AssignmentFactory.cpp b/assignment-client/src/AssignmentFactory.cpp index 0b8e089c01..ef587c9b6d 100644 --- a/assignment-client/src/AssignmentFactory.cpp +++ b/assignment-client/src/AssignmentFactory.cpp @@ -21,10 +21,12 @@ ThreadedAssignment* AssignmentFactory::unpackAssignment(const QByteArray& packet) { int headerBytes = numBytesForPacketHeader(packet); - Assignment::Type assignmentType = Assignment::AllTypes; - memcpy(&assignmentType, packet.data() + headerBytes, sizeof(Assignment::Type)); + quint8 packedType; + memcpy(&packedType, packet.data() + headerBytes, sizeof(packedType)); - switch (assignmentType) { + Assignment::Type unpackedType = (Assignment::Type) packedType; + + switch (unpackedType) { case Assignment::AudioMixerType: return new AudioMixer(packet); case Assignment::AvatarMixerType: diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 073f788f86..665ef45eb1 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -222,6 +222,7 @@ void DomainServer::readAvailableDatagrams() { if (assignmentToDeploy) { // give this assignment out, either the type matches or the requestor said they will take any assignmentPacket.resize(numAssignmentPacketHeaderBytes); + QDataStream assignmentStream(&assignmentPacket, QIODevice::Append); assignmentStream << *assignmentToDeploy; diff --git a/libraries/shared/src/Assignment.cpp b/libraries/shared/src/Assignment.cpp index 0040fd47a3..ca6a6adca7 100644 --- a/libraries/shared/src/Assignment.cpp +++ b/libraries/shared/src/Assignment.cpp @@ -70,6 +70,7 @@ Assignment::Assignment(Assignment::Command command, Assignment::Type type, const } Assignment::Assignment(const QByteArray& packet) : + _pool(), _location(GlobalLocation), _numberOfInstances(1), _payload() @@ -85,7 +86,9 @@ Assignment::Assignment(const QByteArray& packet) : QDataStream packetStream(packet); packetStream.skipRawData(numBytesForPacketHeader(packet)); - packetStream.readRawData(reinterpret_cast(&_type), sizeof(Assignment::Type)); + uchar assignmentType; + packetStream >> assignmentType; + _type = (Assignment::Type) assignmentType; if (_command != Assignment::RequestCommand) { // read the GUID for this assignment @@ -155,13 +158,13 @@ const char* Assignment::getTypeName() const { } QDebug operator<<(QDebug debug, const Assignment &assignment) { - debug.nospace() << "UUID: " << assignment.getUUID().toString().toStdString().c_str() << + debug.nospace() << "UUID: " << qPrintable(assignment.getUUID().toString()) << ", Type: " << assignment.getType(); return debug.nospace(); } QDataStream& operator<<(QDataStream &out, const Assignment& assignment) { - out << (char) assignment._type; + out << (quint8) assignment._type; // pack the UUID for this assignment, if this is an assignment create or deploy if (assignment._command != Assignment::RequestCommand) { diff --git a/libraries/shared/src/PacketHeaders.cpp b/libraries/shared/src/PacketHeaders.cpp index 529f2639d7..fc9511622f 100644 --- a/libraries/shared/src/PacketHeaders.cpp +++ b/libraries/shared/src/PacketHeaders.cpp @@ -16,7 +16,7 @@ #include "PacketHeaders.h" int arithmeticCodingValueFromBuffer(const char* checkValue) { - if (((int) *checkValue) < 255) { + if (((uchar) *checkValue) < 255) { return *checkValue; } else { return 255 + arithmeticCodingValueFromBuffer(checkValue + 1); @@ -24,8 +24,8 @@ int arithmeticCodingValueFromBuffer(const char* checkValue) { } int numBytesArithmeticCodingFromBuffer(const char* checkValue) { - if (((int) *checkValue) < 255) { - return *checkValue; + if (((uchar) *checkValue) < 255) { + return 1; } else { return 1 + numBytesArithmeticCodingFromBuffer(checkValue + 1); } @@ -50,13 +50,19 @@ PacketVersion versionForPacketType(PacketType type) { } } +const int MAX_HEADER_BYTES = sizeof(PacketType) + sizeof(PacketVersion) + NUM_BYTES_RFC4122_UUID; + QByteArray byteArrayWithPopluatedHeader(PacketType type, const QUuid& connectionUUID) { - QByteArray freshByteArray; - populatePacketHeader(freshByteArray, type, connectionUUID); + QByteArray freshByteArray(MAX_HEADER_BYTES, 0); + freshByteArray.resize(populatePacketHeader(freshByteArray, type, connectionUUID)); return freshByteArray; } int populatePacketHeader(QByteArray& packet, PacketType type, const QUuid& connectionUUID) { + if (packet.size() < MAX_HEADER_BYTES) { + packet.resize(numBytesForPacketHeaderGivenPacketType(type)); + } + return populatePacketHeader(packet.data(), type, connectionUUID); } @@ -110,7 +116,7 @@ int numBytesForPacketHeaderGivenPacketType(PacketType type) { } void deconstructPacketHeader(const QByteArray& packet, QUuid& senderUUID) { - senderUUID = QUuid::fromRfc4122(packet.mid(arithmeticCodingValueFromBuffer(packet.data()) + sizeof(PacketVersion))); + senderUUID = QUuid::fromRfc4122(packet.mid(numBytesArithmeticCodingFromBuffer(packet.data()) + sizeof(PacketVersion))); } PacketType packetTypeForPacket(const QByteArray& packet) {