Remove 1500+ memory leaks in OpenSpace and Ghoul

modules/base/rendering/renderablemodel.cpp

@@ -148,12 +148,13 @@ bool RenderableModel::deinitialize() {
 	if (_geometry) {
 		_geometry->deinitialize();
 		delete _geometry;
+        _geometry = nullptr;
 	}
-	if (_texture)
-		delete _texture;
+	delete _texture;
+    _texture = nullptr;
 
-	_geometry = nullptr;
-	_texture = nullptr;
+    delete _programObject;
+    _programObject = nullptr;
 	return true;
 }
 

modules/base/rendering/renderablepath.cpp

@@ -119,7 +119,14 @@ bool RenderablePath::initialize() {
 
 bool RenderablePath::deinitialize() {
 	glDeleteVertexArrays(1, &_vaoID);
+    _vaoID = 0;
+
 	glDeleteBuffers(1, &_vBufferID);
+    _vBufferID = 0;
+
+    delete _programObject;
+    _programObject = nullptr;
+
 	return true;
 }
 

modules/base/rendering/renderableplane.cpp

@@ -109,6 +109,7 @@ RenderablePlane::RenderablePlane(const ghoul::Dictionary& dictionary)
 
 RenderablePlane::~RenderablePlane() {
     delete _textureFile;
+    _textureFile = nullptr;
 }
 
 bool RenderablePlane::isReady() const {
@@ -142,9 +143,19 @@ bool RenderablePlane::initialize() {
 bool RenderablePlane::deinitialize() {
 	glDeleteVertexArrays(1, &_quad);
 	_quad = 0;
+
 	glDeleteBuffers(1, &_vertexPositionBuffer);
 	_vertexPositionBuffer = 0;
+
 	delete _texture;
+    _texture = nullptr;
+
+    delete _textureFile;
+    _textureFile = nullptr;
+
+    delete _shader;
+    _shader = nullptr;
+
 	return true;
 }
 

modules/base/rendering/renderableplane.h

@@ -52,7 +52,7 @@ class RenderablePlane : public Renderable {
 
 public:
 	RenderablePlane(const ghoul::Dictionary& dictionary);
-	~RenderablePlane();
+    ~RenderablePlane();
 
 	bool initialize() override;
 	bool deinitialize() override;

modules/base/rendering/renderablesphere.cpp

@@ -106,9 +106,6 @@ RenderableSphere::RenderableSphere(const ghoul::Dictionary& dictionary)
 	_texturePath.onChange(std::bind(&RenderableSphere::loadTexture, this));
 }
 
-RenderableSphere::~RenderableSphere() {
-}
-
 bool RenderableSphere::isReady() const {
     return (_sphere != nullptr) && (_shader != nullptr) && (_texture != nullptr);
 }
@@ -132,7 +129,14 @@ bool RenderableSphere::initialize() {
 
 bool RenderableSphere::deinitialize() {
     delete _sphere;
+    _sphere = nullptr;
+
 	delete _texture;
+    _texture = nullptr;
+
+    delete _shader;
+    _shader = nullptr;
+
 	return true;
 }
 

modules/base/rendering/renderablesphere.h

@@ -41,7 +41,6 @@ class PowerScaledSphere;
 class RenderableSphere : public Renderable {
 public:
 	RenderableSphere(const ghoul::Dictionary& dictionary);
-	~RenderableSphere();
 
 	bool initialize() override;
 	bool deinitialize() override;

modules/base/rendering/renderablestars.cpp

@@ -137,6 +137,7 @@ RenderableStars::RenderableStars(const ghoul::Dictionary& dictionary)
 RenderableStars::~RenderableStars() {
     delete _psfTextureFile;
     delete _colorTextureFile;
+    delete _colorTexture;
 }
 
 bool RenderableStars::isReady() const {

modules/base/rendering/renderabletrail.cpp

@@ -100,6 +100,10 @@ RenderableTrail::RenderableTrail(const ghoul::Dictionary& dictionary)
 	_distanceFade = 1.0;
 }
 
+RenderableTrail::~RenderableTrail() {
+    delete _programObject;
+}
+
 bool RenderableTrail::initialize() {
     if (!_successfullDictionaryFetch) {
         LERROR("The following keys need to be set in the Dictionary. Cannot initialize!");

modules/base/rendering/renderabletrail.h

@@ -43,6 +43,7 @@ namespace openspace {
 class RenderableTrail : public Renderable {
 public:
     RenderableTrail(const ghoul::Dictionary& dictionary);
+    ~RenderableTrail();
 
     bool initialize() override;
     bool deinitialize() override;

openspace.cfg

@@ -53,6 +53,6 @@ return {
         Type = "text",
         File = "${BASE_PATH}/Properties.txt"
     },
-    RenderingMethod = "ABufferSingleLinked" -- On Windows and Unix
-    -- RenderingMethod = "ABufferFrameBuffer" -- On Mac due to OpenGL 4.1 restrictions
+    -- RenderingMethod = "ABufferSingleLinked" -- On Windows and Unix
+    RenderingMethod = "ABufferFrameBuffer" -- On Mac due to OpenGL 4.1 restrictions
 }

src/engine/moduleengine.cpp

@@ -60,6 +60,7 @@ bool ModuleEngine::deinitialize() {
             LERROR("Could not deinitialize module '" << m->name() << "'");
             return false;
         }
+        delete m;
     }
     LDEBUG("Finished Deinitializing modules");
     return true;

src/engine/openspaceengine.cpp

@@ -49,7 +49,7 @@
 #include <openspace/util/syncbuffer.h>
 #include <openspace/engine/moduleengine.h>
 
-
+#include <ghoul/ghoul.h>
 #include <ghoul/cmdparser/commandlineparser.h>
 #include <ghoul/cmdparser/singlecommand.h>
 #include <ghoul/filesystem/filesystem.h>
@@ -148,6 +148,8 @@ bool OpenSpaceEngine::create(
     std::vector<std::string>& sgctArguments,
     std::string& openGlVersion)
 {
+    ghoul::initialize();
+
     ghoul_assert(!_engine, "OpenSpaceEngine was already created");
 
 	// Initialize the LogManager and add the console log as this will be used every time
@@ -271,6 +273,8 @@ bool OpenSpaceEngine::create(
 void OpenSpaceEngine::destroy() {
     _engine->_moduleEngine->deinitialize();
     _engine->_console->deinitialize();
+
+    _engine->_scriptEngine->deinitialize();
 	delete _engine;
 	ghoul::systemcapabilities::SystemCapabilities::deinitialize();
 	FactoryManager::deinitialize();
@@ -279,6 +283,8 @@ void OpenSpaceEngine::destroy() {
 
 	FileSystem::deinitialize();
 	LogManager::deinitialize();
+
+    ghoul::deinitialize();
 }
 
 bool OpenSpaceEngine::initialize() {

src/main.cpp

@@ -53,8 +53,10 @@ void setupPostFX();
 GLint _postFXTexLoc;
 GLint _postFXOpacityLoc;
 
+#include <ghoul/lua/lua_helper.h>
+
 namespace {
-	const std::string _loggerCat = "main";
+    const std::string _loggerCat = "main";
 }
 
 int main(int argc, char** argv) {
@@ -76,16 +78,16 @@ int main(int argc, char** argv) {
         newArgv[i] = const_cast<char*>(sgctArguments.at(i).c_str());
 
     // Need to set this before the creation of the sgct::Engine
-	sgct::MessageHandler::instance()->setLogToConsole(false);
-	sgct::MessageHandler::instance()->setShowTime(false);
-	sgct::MessageHandler::instance()->setLogToCallback(true);
-	sgct::MessageHandler::instance()->setLogCallback(mainLogCallback);
+    sgct::MessageHandler::instance()->setLogToConsole(false);
+    sgct::MessageHandler::instance()->setShowTime(false);
+    sgct::MessageHandler::instance()->setLogToCallback(true);
+    sgct::MessageHandler::instance()->setLogCallback(mainLogCallback);
 
 #ifdef __APPLE__
     glfwWindowHint(GLFW_STENCIL_BITS, 8);
 #endif
     
-	LDEBUG("Creating SGCT Engine");
+    LDEBUG("Creating SGCT Engine");
     _sgctEngine = new sgct::Engine(newArgc, newArgv);
 
     // deallocate sgct c arguments
@@ -100,9 +102,9 @@ int main(int argc, char** argv) {
     _sgctEngine->setKeyboardCallbackFunction(mainKeyboardCallback);
     _sgctEngine->setMouseButtonCallbackFunction(mainMouseButtonCallback);
     _sgctEngine->setMousePosCallbackFunction(mainMousePosCallback);
-	_sgctEngine->setMouseScrollCallbackFunction(mainMouseScrollCallback);
-	_sgctEngine->setExternalControlCallback(mainExternalControlCallback);
-	_sgctEngine->setCharCallbackFunction(mainCharCallback);
+    _sgctEngine->setMouseScrollCallbackFunction(mainMouseScrollCallback);
+    _sgctEngine->setExternalControlCallback(mainExternalControlCallback);
+    _sgctEngine->setCharCallbackFunction(mainCharCallback);
 
     // set encode and decode functions
     // NOTE: starts synchronizing before init functions
@@ -110,9 +112,9 @@ int main(int argc, char** argv) {
     sgct::SharedData::instance()->setDecodeFunction(mainDecodeFun);
 
     // try to open a window
-	LDEBUG("Initialize SGCT Engine");
+    LDEBUG("Initialize SGCT Engine");
 #ifdef __APPLE__
-	sgct::Engine::RunMode rm = sgct::Engine::RunMode::OpenGL_4_1_Core_Profile;
+    sgct::Engine::RunMode rm = sgct::Engine::RunMode::OpenGL_4_1_Core_Profile;
 #else
     std::map<std::string, sgct::Engine::RunMode> versionMapping = {
         { "4.2", sgct::Engine::RunMode::OpenGL_4_2_Core_Profile },
@@ -126,27 +128,27 @@ int main(int argc, char** argv) {
     }
     sgct::Engine::RunMode rm = versionMapping[openGlVersion];
 #endif
-	const bool initSuccess = _sgctEngine->init(rm);
+    const bool initSuccess = _sgctEngine->init(rm);
     if (!initSuccess) {
-		LFATAL("Initializing failed");
+        LFATAL("Initializing failed");
         // could not open a window, deallocates and exits
         delete _sgctEngine;
         openspace::OpenSpaceEngine::destroy();
         return EXIT_FAILURE;
     }
 
-	//is this node the master?	(must be set after call to _sgctEngine->init())
-	OsEng.setMaster(_sgctEngine->isMaster());
+    //is this node the master?	(must be set after call to _sgctEngine->init())
+    OsEng.setMaster(_sgctEngine->isMaster());
 
     // Main loop
-	LDEBUG("Starting rendering loop");
+    LDEBUG("Starting rendering loop");
     _sgctEngine->render();
 
-	LDEBUG("Destroying OpenSpaceEngine");
-	openspace::OpenSpaceEngine::destroy();
+    LDEBUG("Destroying OpenSpaceEngine");
+    openspace::OpenSpaceEngine::destroy();
 
     // Clean up (de-allocate)
-	LDEBUG("Destroying SGCT Engine");
+    LDEBUG("Destroying SGCT Engine");
     delete _sgctEngine;
 
     // Exit program
@@ -154,44 +156,44 @@ int main(int argc, char** argv) {
 }
 
 void mainInitFunc() {
-	bool success = OsEng.initialize();
-	if (success)
-		success = OsEng.initializeGL();
+    bool success = OsEng.initialize();
+    if (success)
+        success = OsEng.initializeGL();
 
-	if (!success) {
-		LFATAL("Initializing OpenSpaceEngine failed");
-		std::cout << "Press any key to continue...";
-		std::cin.ignore(100);
-		exit(EXIT_FAILURE);
-	}
+    if (!success) {
+        LFATAL("Initializing OpenSpaceEngine failed");
+        std::cout << "Press any key to continue...";
+        std::cin.ignore(100);
+        exit(EXIT_FAILURE);
+    }
 
-	//temporary post-FX solution, TODO add a more permanent solution @JK
-	setupPostFX();
+    //temporary post-FX solution, TODO add a more permanent solution @JK
+    setupPostFX();
 }
 
 void mainPreSyncFunc() {
-	OsEng.preSynchronization();
+    OsEng.preSynchronization();
 }
 
 void mainPostSyncPreDrawFunc() {
-	OsEng.postSynchronizationPreDraw();
+    OsEng.postSynchronizationPreDraw();
 }
 
 void mainRenderFunc() {
     using glm::mat4;
     using glm::translate;
-	//not the most efficient, but for clarity @JK
-	
-	mat4 userMatrix = translate(mat4(1.f), _sgctEngine->getDefaultUserPtr()->getPos());
-	mat4 sceneMatrix = _sgctEngine->getModelMatrix();
-	mat4 viewMatrix = _sgctEngine->getActiveViewMatrix() * userMatrix;
-	
-	//dont shift nav-direction on master, makes it very tricky to navigate @JK
-	if (!OsEng.ref().isMaster())
-		viewMatrix = viewMatrix * sceneMatrix;
+    //not the most efficient, but for clarity @JK
+    
+    mat4 userMatrix = translate(mat4(1.f), _sgctEngine->getDefaultUserPtr()->getPos());
+    mat4 sceneMatrix = _sgctEngine->getModelMatrix();
+    mat4 viewMatrix = _sgctEngine->getActiveViewMatrix() * userMatrix;
+    
+    //dont shift nav-direction on master, makes it very tricky to navigate @JK
+    if (!OsEng.ref().isMaster())
+        viewMatrix = viewMatrix * sceneMatrix;
 
-	mat4 projectionMatrix = _sgctEngine->getActiveProjectionMatrix();
-	OsEng.render(projectionMatrix, viewMatrix);
+    mat4 projectionMatrix = _sgctEngine->getActiveProjectionMatrix();
+    OsEng.render(projectionMatrix, viewMatrix);
 }
 
 void mainPostDrawFunc() {
@@ -200,7 +202,7 @@ void mainPostDrawFunc() {
 
 void mainExternalControlCallback(const char* receivedChars, int size) {
     if (OsEng.isMaster())
-		OsEng.externalControlCallback(receivedChars, size, 0);
+        OsEng.externalControlCallback(receivedChars, size, 0);
 }
 
 void mainKeyboardCallback(int key, int action) {
@@ -224,8 +226,8 @@ void mainMouseScrollCallback(double posX, double posY) {
 }
 
 void mainCharCallback(unsigned int codepoint) {
-	if (OsEng.isMaster())
-		OsEng.charCallback(codepoint);
+    if (OsEng.isMaster())
+        OsEng.charCallback(codepoint);
 }
 
 void mainEncodeFun() {
@@ -237,30 +239,30 @@ void mainDecodeFun() {
 }
 
 void mainLogCallback(const char* msg){
-	std::string message = msg;
-	// Remove the trailing \n that is passed along
-	LINFOC("SGCT", message.substr(0, std::max<size_t>(message.size() - 1, 0)));
+    std::string message = msg;
+    // Remove the trailing \n that is passed along
+    LINFOC("SGCT", message.substr(0, std::max<size_t>(message.size() - 1, 0)));
 }
 
 void postFXPass(){
-	glUniform1i(_postFXTexLoc, 0);
+    glUniform1i(_postFXTexLoc, 0);
     if (OsEng.isMaster())
         glUniform1f(_postFXOpacityLoc, 1.f);
     else
-		glUniform1f(_postFXOpacityLoc, OsEng.renderEngine()->globalBlackOutFactor());
+        glUniform1f(_postFXOpacityLoc, OsEng.renderEngine()->globalBlackOutFactor());
 }
 
 void setupPostFX(){
 #ifndef __APPLE__
-	sgct::PostFX fx[1];
-	sgct::ShaderProgram *shader;
-	fx[0].init("OpacityControl", absPath("${SHADERS}/postFX_vs.glsl"), absPath("${SHADERS}/postFX_fs.glsl"));
-	fx[0].setUpdateUniformsFunction(postFXPass);
-	shader = fx[0].getShaderProgram();
-	shader->bind();
-	_postFXTexLoc = shader->getUniformLocation("Tex");
-	_postFXOpacityLoc = shader->getUniformLocation("Opacity");
-	shader->unbind();
-	_sgctEngine->addPostFX(fx[0]);
+    sgct::PostFX fx[1];
+    sgct::ShaderProgram *shader;
+    fx[0].init("OpacityControl", absPath("${SHADERS}/postFX_vs.glsl"), absPath("${SHADERS}/postFX_fs.glsl"));
+    fx[0].setUpdateUniformsFunction(postFXPass);
+    shader = fx[0].getShaderProgram();
+    shader->bind();
+    _postFXTexLoc = shader->getUniformLocation("Tex");
+    _postFXOpacityLoc = shader->getUniformLocation("Opacity");
+    shader->unbind();
+    _sgctEngine->addPostFX(fx[0]);
 #endif
 }

src/properties/propertyowner.cpp

@@ -50,19 +50,16 @@ PropertyOwner::PropertyOwner()
 {
 }
 
-PropertyOwner::~PropertyOwner()
-{
+PropertyOwner::~PropertyOwner() {
 	_properties.clear();
 	_subOwners.clear();
 }
 
-const std::vector<Property*>& PropertyOwner::properties() const
-{
+const std::vector<Property*>& PropertyOwner::properties() const {
     return _properties;
 }
 
-std::vector<Property*> PropertyOwner::propertiesRecursive() const
-{
+std::vector<Property*> PropertyOwner::propertiesRecursive() const {
 	std::vector<Property*> props = properties();
 
 	for (const PropertyOwner* owner : _subOwners) {
@@ -73,8 +70,7 @@ std::vector<Property*> PropertyOwner::propertiesRecursive() const
 	return std::move(props);
 }
 
-Property* PropertyOwner::property(const std::string& id) const
-{
+Property* PropertyOwner::property(const std::string& id) const {
     assert(std::is_sorted(_properties.begin(), _properties.end(), propertyLess));
 
 	// As the _properties list is sorted, just finding the lower bound is sufficient
@@ -138,13 +134,11 @@ bool PropertyOwner::hasPropertySubOwner(const std::string& name) const {
     return propertySubOwner(name) != nullptr;
 }
 
-void PropertyOwner::setPropertyGroupName(std::string groupID, std::string name)
-{
+void PropertyOwner::setPropertyGroupName(std::string groupID, std::string name) {
     _groupNames[std::move(groupID)] = std::move(name);
 }
     
-const std::string& PropertyOwner::propertyGroupName(const std::string& groupID) const
-{
+const std::string& PropertyOwner::propertyGroupName(const std::string& groupID) const {
     auto it = _groupNames.find(groupID);
     if (it == _groupNames.end())
         return groupID;
@@ -193,8 +187,7 @@ void PropertyOwner::addProperty(Property* prop)
     }
 }
 
-void PropertyOwner::addProperty(Property& prop)
-{
+void PropertyOwner::addProperty(Property& prop) {
     addProperty(&prop);
 }
     
@@ -241,8 +234,7 @@ void PropertyOwner::addPropertySubOwner(openspace::properties::PropertyOwner& ow
     addPropertySubOwner(&owner);
 }
 
-void PropertyOwner::removeProperty(Property* prop)
-{
+void PropertyOwner::removeProperty(Property* prop) {
     assert(prop != nullptr);
 
     // See if we can find the identifier of the property to add in the properties list
@@ -261,8 +253,7 @@ void PropertyOwner::removeProperty(Property* prop)
                                             << "' not found for removal.");
 }
 
-void PropertyOwner::removeProperty(Property& prop)
-{
+void PropertyOwner::removeProperty(Property& prop) {
     removeProperty(&prop);
 }
     
@@ -288,13 +279,11 @@ void PropertyOwner::removePropertySubOwner(openspace::properties::PropertyOwner&
     removePropertySubOwner(&owner);
 }
 
-void PropertyOwner::setName(std::string name)
-{
+void PropertyOwner::setName(std::string name) {
     _name = std::move(name);
 }
 
-const std::string& PropertyOwner::name() const
-{
+const std::string& PropertyOwner::name() const {
     return _name;
 }
 

src/scene/scenegraph.cpp

@@ -33,6 +33,12 @@
 
 #include <stack>
 
+#ifdef _MSC_VER
+#ifdef OPENSPACE_ENABLE_VLD
+#include <vld.h>
+#endif
+#endif
+
 namespace {
     const std::string _loggerCat = "SceneGraph";
     const std::string _moduleExtension = ".mod";
@@ -169,7 +175,13 @@ bool SceneGraph::loadFromFile(const std::string& sceneDescription) {
         }
 
         ghoul::Dictionary moduleDictionary;
+//#ifdef OPENSPACE_ENABLE_VLD
+//        VLDDisable();
+//#endif
         bool s = ghoul::lua::loadDictionaryFromFile(moduleFile, moduleDictionary, state);
+//#ifdef OPENSPACE_ENABLE_VLD
+//        VLDEnable();
+//#endif
         if (!s)
             continue;
 
@@ -226,6 +238,7 @@ bool SceneGraph::loadFromFile(const std::string& sceneDescription) {
             _nodes.push_back(internalNode);
         }
     }
+    ghoul::lua::destroyLuaState(state);
     FileSys.setCurrentDirectory(oldDirectory);
 
     for (SceneGraphNodeInternal* node : _nodes) {

src/scene/scenegraphnode.cpp

@@ -63,6 +63,7 @@ SceneGraphNode* SceneGraphNode::createFromDictionary(const ghoul::Dictionary& di
 
     if (!dictionary.hasValue<std::string>(KeyName)) {
         LERROR("SceneGraphNode did not contain a '" << KeyName << "' key");
+        delete result;
         return nullptr;
     }
     std::string name;