From 131a57248b694d1cfedd9ef744207c1aba09aee3 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 8 Jun 2018 10:27:03 -0700 Subject: [PATCH 1/5] NLPacket local ID could be used uninitialized (according to valgrind) --- libraries/networking/src/NLPacket.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/NLPacket.cpp b/libraries/networking/src/NLPacket.cpp index f946e97bf4..620e60945b 100644 --- a/libraries/networking/src/NLPacket.cpp +++ b/libraries/networking/src/NLPacket.cpp @@ -199,7 +199,9 @@ void NLPacket::readVersion() { } void NLPacket::readSourceID() { - if (!PacketTypeEnum::getNonSourcedPackets().contains(_type)) { + if (PacketTypeEnum::getNonSourcedPackets().contains(_type)) { + _sourceID = NULL_LOCAL_ID; + } else { _sourceID = sourceIDInHeader(*this); } } From e3c8895c8965e0128d002d12aef36aecedd0cdc8 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 8 Jun 2018 18:17:03 -0700 Subject: [PATCH 2/5] Take down DependencyManager upon restart --- assignment-client/src/scripts/EntityScriptServer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index eea8e8b470..5722cccf38 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -570,6 +570,8 @@ void EntityScriptServer::aboutToFinish() { entityScriptingInterface->setPacketSender(nullptr); } + DependencyManager::destroy(); + DependencyManager::get()->cleanup(); // cleanup the AudioInjectorManager (and any still running injectors) From 5ae79bcc5e4f4613b42629b9ca3a53e80dda8dd6 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 11 Jun 2018 13:05:42 -0700 Subject: [PATCH 3/5] Use weak_ptr for pointers back to parent --- libraries/entities/src/EntityTreeElement.cpp | 6 +++--- libraries/entities/src/EntityTreeElement.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index cbcddfc57b..719c065522 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -33,7 +33,7 @@ EntityTreeElement::~EntityTreeElement() { OctreeElementPointer EntityTreeElement::createNewElement(unsigned char* octalCode) { auto newChild = EntityTreeElementPointer(new EntityTreeElement(octalCode)); - newChild->setTree(_myTree); + newChild->setTree(getTree()); return newChild; } @@ -44,7 +44,7 @@ void EntityTreeElement::init(unsigned char* octalCode) { OctreeElementPointer EntityTreeElement::addChildAtIndex(int index) { OctreeElementPointer newElement = OctreeElement::addChildAtIndex(index); - std::static_pointer_cast(newElement)->setTree(_myTree); + std::static_pointer_cast(newElement)->setTree(getTree()); return newElement; } @@ -475,7 +475,7 @@ bool EntityTreeElement::removeEntityItem(EntityItemPointer entity, bool deletion int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int bytesLeftToRead, ReadBitstreamToTreeParams& args) { - return _myTree->readEntityDataFromBuffer(data, bytesLeftToRead, args); + return getTree()->readEntityDataFromBuffer(data, bytesLeftToRead, args); } void EntityTreeElement::addEntityItem(EntityItemPointer entity) { diff --git a/libraries/entities/src/EntityTreeElement.h b/libraries/entities/src/EntityTreeElement.h index 76e1e40812..f9f930a9fc 100644 --- a/libraries/entities/src/EntityTreeElement.h +++ b/libraries/entities/src/EntityTreeElement.h @@ -161,8 +161,8 @@ public: virtual uint16_t size() const; bool hasEntities() const { return size() > 0; } - void setTree(EntityTreePointer tree) { _myTree = tree; } - EntityTreePointer getTree() const { return _myTree; } + void setTree(EntityTreePointer tree) { _myTree.swap(std::weak_ptr(tree)); } + EntityTreePointer getTree() const { return _myTree.lock(); } void addEntityItem(EntityItemPointer entity); @@ -234,7 +234,7 @@ public: protected: virtual void init(unsigned char * octalCode) override; - EntityTreePointer _myTree; + std::weak_ptr _myTree; EntityItems _entityItems; }; From 99a8ecc6db9865d63278e0a59f29f607ffb1d2f8 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 11 Jun 2018 13:52:44 -0700 Subject: [PATCH 4/5] Break simulation/entity-tree cycle --- assignment-client/src/entities/EntityTreeHeadlessViewer.cpp | 3 +++ libraries/entities/src/EntityTreeElement.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp b/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp index 81c42cda1e..3649cf1129 100644 --- a/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp +++ b/assignment-client/src/entities/EntityTreeHeadlessViewer.cpp @@ -17,6 +17,9 @@ EntityTreeHeadlessViewer::EntityTreeHeadlessViewer() } EntityTreeHeadlessViewer::~EntityTreeHeadlessViewer() { + if (_simulation) { + _simulation->setEntityTree(nullptr); // Break shared_ptr cycle. + } } void EntityTreeHeadlessViewer::init() { diff --git a/libraries/entities/src/EntityTreeElement.h b/libraries/entities/src/EntityTreeElement.h index f9f930a9fc..023e908e2c 100644 --- a/libraries/entities/src/EntityTreeElement.h +++ b/libraries/entities/src/EntityTreeElement.h @@ -161,7 +161,7 @@ public: virtual uint16_t size() const; bool hasEntities() const { return size() > 0; } - void setTree(EntityTreePointer tree) { _myTree.swap(std::weak_ptr(tree)); } + void setTree(EntityTreePointer tree) { _myTree = std::weak_ptr(tree); } EntityTreePointer getTree() const { return _myTree.lock(); } void addEntityItem(EntityItemPointer entity); From 9ae3411abea821b5743e91dff30b02e86eb3dfa8 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 12 Jun 2018 12:32:27 -0700 Subject: [PATCH 5/5] Have ~OctreeProcessor break cycle instead of using weak pointers --- libraries/entities/src/EntityTreeElement.cpp | 6 +++--- libraries/entities/src/EntityTreeElement.h | 6 +++--- libraries/octree/src/OctreeProcessor.cpp | 9 +++------ libraries/octree/src/OctreeProcessor.h | 3 +-- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 719c065522..cbcddfc57b 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -33,7 +33,7 @@ EntityTreeElement::~EntityTreeElement() { OctreeElementPointer EntityTreeElement::createNewElement(unsigned char* octalCode) { auto newChild = EntityTreeElementPointer(new EntityTreeElement(octalCode)); - newChild->setTree(getTree()); + newChild->setTree(_myTree); return newChild; } @@ -44,7 +44,7 @@ void EntityTreeElement::init(unsigned char* octalCode) { OctreeElementPointer EntityTreeElement::addChildAtIndex(int index) { OctreeElementPointer newElement = OctreeElement::addChildAtIndex(index); - std::static_pointer_cast(newElement)->setTree(getTree()); + std::static_pointer_cast(newElement)->setTree(_myTree); return newElement; } @@ -475,7 +475,7 @@ bool EntityTreeElement::removeEntityItem(EntityItemPointer entity, bool deletion int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int bytesLeftToRead, ReadBitstreamToTreeParams& args) { - return getTree()->readEntityDataFromBuffer(data, bytesLeftToRead, args); + return _myTree->readEntityDataFromBuffer(data, bytesLeftToRead, args); } void EntityTreeElement::addEntityItem(EntityItemPointer entity) { diff --git a/libraries/entities/src/EntityTreeElement.h b/libraries/entities/src/EntityTreeElement.h index 023e908e2c..76e1e40812 100644 --- a/libraries/entities/src/EntityTreeElement.h +++ b/libraries/entities/src/EntityTreeElement.h @@ -161,8 +161,8 @@ public: virtual uint16_t size() const; bool hasEntities() const { return size() > 0; } - void setTree(EntityTreePointer tree) { _myTree = std::weak_ptr(tree); } - EntityTreePointer getTree() const { return _myTree.lock(); } + void setTree(EntityTreePointer tree) { _myTree = tree; } + EntityTreePointer getTree() const { return _myTree; } void addEntityItem(EntityItemPointer entity); @@ -234,7 +234,7 @@ public: protected: virtual void init(unsigned char * octalCode) override; - std::weak_ptr _myTree; + EntityTreePointer _myTree; EntityItems _entityItems; }; diff --git a/libraries/octree/src/OctreeProcessor.cpp b/libraries/octree/src/OctreeProcessor.cpp index db78e985e6..beaac1198c 100644 --- a/libraries/octree/src/OctreeProcessor.cpp +++ b/libraries/octree/src/OctreeProcessor.cpp @@ -20,12 +20,6 @@ #include "OctreeLogging.h" -OctreeProcessor::OctreeProcessor() : - _tree(NULL), - _managedTree(false) -{ -} - void OctreeProcessor::init() { if (!_tree) { _tree = createTree(); @@ -34,6 +28,9 @@ void OctreeProcessor::init() { } OctreeProcessor::~OctreeProcessor() { + if (_tree) { + _tree->eraseAllOctreeElements(false); + } } void OctreeProcessor::setTree(OctreePointer newTree) { diff --git a/libraries/octree/src/OctreeProcessor.h b/libraries/octree/src/OctreeProcessor.h index 25e280abca..325b33cd15 100644 --- a/libraries/octree/src/OctreeProcessor.h +++ b/libraries/octree/src/OctreeProcessor.h @@ -28,7 +28,6 @@ class OctreeProcessor : public QObject, public QEnableSharedFromThis { Q_OBJECT public: - OctreeProcessor(); virtual ~OctreeProcessor(); virtual char getMyNodeType() const = 0; @@ -61,7 +60,7 @@ protected: virtual OctreePointer createTree() = 0; OctreePointer _tree; - bool _managedTree; + bool _managedTree { false }; SimpleMovingAverage _elementsPerPacket; SimpleMovingAverage _entitiesPerPacket;