Fixed missing thread-safety of cgt::TextureUnit and cgt::ResourceManager.

Both classes now have a mutex protecting its shared fields. One global mutex may not be the most efficient implementation but at least is reliable and easy to follow.

closes #598

ext/cgt/manager.h

@@ -35,6 +35,8 @@
 #include <list>
 #include <iostream>
 
+#include <tbb/spin_mutex.h>
+
 #include "cgt/types.h"
 #include "cgt/assert.h"
 #include "cgt/filesystem.h"
@@ -86,6 +88,8 @@ protected:
 
     std::list<std::string> pathList_;
 
+    tbb::spin_mutex mutex_;
+
     void reg(T *ptr, const std::string& filename);
     void increaseUsage(const std::string& filename);
 
@@ -104,6 +108,7 @@ const std::string ResourceManager<T>::loggerCat_("cgt.Manager");
 
 template <class T>
 T* ResourceManager<T>::get(const std::string& filename) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
     return resourcesByFilename_[filename]->data_;
 }
 
@@ -114,6 +119,8 @@ T* ResourceManager<T>::get(const std::string& filename) {
 
 template <class T>
 void ResourceManager<T>::reg(T* ptr, const std::string& filename) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
+
     if (cacheResources_ && isLoaded(filename)) {
         Resource* r = resourcesByFilename_[filename];
         r->data_ = ptr;
@@ -131,6 +138,7 @@ void ResourceManager<T>::reg(T* ptr, const std::string& filename) {
 
 template <class T>
 void ResourceManager<T>::increaseUsage(const std::string& filename) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
     cgtAssert(cacheResources_, "increaseUsage should not be called in non-caching mode!");
     resourcesByFilename_[filename]->usedBy_++;
 }
@@ -156,11 +164,14 @@ ResourceManager<T>::~ResourceManager() {
 
 template <class T>
 bool ResourceManager<T>::isLoaded(const std::string& filename) {
-    return(cacheResources_ && (resourcesByFilename_.find(filename) != resourcesByFilename_.end()));
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
+    return (cacheResources_ && (resourcesByFilename_.find(filename) != resourcesByFilename_.end()));
 }
 
 template <class T>
 void ResourceManager<T>::dispose(T* ptr) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
+
     if (ptr == 0 || resourcesByPtr_.find(ptr) == resourcesByPtr_.end())
         return;
 
@@ -183,6 +194,8 @@ void ResourceManager<T>::dispose(T* ptr) {
 
 template <class T>
 void ResourceManager<T>::addPath(std::string path) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
+
     pathList_.push_front(path);
     // remove duplicates
     //TODO: better use std::set<> here
@@ -192,6 +205,8 @@ void ResourceManager<T>::addPath(std::string path) {
 
 template <class T>
 void ResourceManager<T>::removePath(std::string path) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
+
     std::list<std::string>::iterator it;
     for (it = pathList_.begin(); it != pathList_.end(); ++it) {
         if (*it == path) {
@@ -203,6 +218,7 @@ void ResourceManager<T>::removePath(std::string path) {
 
 template <class T>
 std::string ResourceManager<T>::completePath(std::string filename) {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
     std::string cplFileName = filename;
 
     if(FileSys.exists(filename))
@@ -230,6 +246,8 @@ std::string ResourceManager<T>::completePath(std::string filename) {
 
 template <class T>
 std::vector<std::string> ResourceManager<T>::getFilenames() {
+    tbb::spin_mutex::scoped_lock lock(this->mutex_);
+
     std::vector<std::string> filenames;
     for (typename std::map<std::string, Resource*>::const_iterator iter = resourcesByFilename_.begin();
          iter != resourcesByFilename_.end(); iter++)

ext/cgt/textureunit.cpp

@@ -37,6 +37,7 @@ unsigned short TextureUnit::totalActive_ = 0;
 unsigned short TextureUnit::maxTexUnits_ = 0;
 unsigned short TextureUnit::numKeptUnits_ = 0;
 std::vector<bool> TextureUnit::busyUnits_ = std::vector<bool>();
+tbb::spin_mutex TextureUnit::mutex_ = tbb::spin_mutex();
 
 TextureUnit::TextureUnit(bool keep)
     : number_(0)
@@ -78,6 +79,8 @@ void TextureUnit::setZeroUnit() {
 }
 
 void TextureUnit::cleanup() {
+    tbb::spin_mutex::scoped_lock lock(mutex_);
+
     for (size_t i = 0; i < maxTexUnits_; i++) {
         if (busyUnits_.at(i))
             busyUnits_.at(i) = false;
@@ -96,6 +99,7 @@ unsigned short TextureUnit::numLocalActive() {
 }
 
 void TextureUnit::assignUnit() const {
+    tbb::spin_mutex::scoped_lock lock(mutex_);
     cgtAssert(totalActive_ <= maxTexUnits_, "No more texture units available");
 
     assigned_ = true;
@@ -115,10 +119,14 @@ void TextureUnit::assignUnit() const {
 }
 
 void TextureUnit::init() {
+    tbb::spin_mutex::scoped_lock lock(mutex_);
+    if (initialized_)
+        return;
+
     maxTexUnits_ = GpuCaps.getNumTextureUnits();
     busyUnits_ = std::vector<bool>(maxTexUnits_, false);
-    initialized_ = true;
     numKeptUnits_ = 0;
+    initialized_ = true;
 }
 
 } // namespace

ext/cgt/textureunit.h

@@ -30,6 +30,8 @@
 #define CGT_TEXTUREUNIT_H
 
 #include <string>
+
+#include <tbb/spin_mutex.h>
 #include "cgt/cgt_gl.h"
 #include "cgt/shadermanager.h"
 
@@ -72,6 +74,7 @@ protected:
     static unsigned short maxTexUnits_;
     static unsigned short numKeptUnits_;
     static std::vector<bool> busyUnits_;
+    static tbb::spin_mutex mutex_;
 };
 
 } // namespace cgt

test/CMakeLists.txt

@@ -32,7 +32,7 @@ ADD_EXECUTABLE(test-campvis
 
 ADD_DEFINITIONS(${CampvisGlobalDefinitions} ${CampvisModulesDefinitions} ${CampvisApplicationDefinitions} ${QT_DEFINITIONS})
 INCLUDE_DIRECTORIES(${CampvisGlobalIncludeDirs} ${CampvisModulesIncludeDirs} ${CampvisHome}/ext/gtest-1.7.0/ ${CampvisHome}/ext/gtest-1.7.0/include)
-TARGET_LINK_LIBRARIES(test-campvis campvis-modules campvis-core cgt gtest sigslot ${CampvisGlobalExternalLibs} ${CampvisModulesExternalLibs} ${QT_LIBRARIES})
+TARGET_LINK_LIBRARIES(test-campvis gtest sigslot cgt campvis-core campvis-modules ${CampvisGlobalExternalLibs} ${CampvisModulesExternalLibs} ${QT_LIBRARIES})
 
 if (CAMPVIS_GROUP_SOURCE_FILES)
     DEFINE_SOURCE_GROUPS_FROM_SUBDIR(TestCampvisSources ${CampvisHome}/test "")