The expiration time for new job artifacts in CI/CD pipelines is now 30 days (GitLab default). Previously generated artifacts in already completed jobs will not be affected by the change. The latest artifacts for all jobs in the latest successful pipelines will be kept. More information: https://gitlab.lrz.de/help/user/admin_area/settings/continuous_integration.html#default-artifacts-expiration

Commit ac28b5d7 authored by Christian Schulte zu Berge's avatar Christian Schulte zu Berge
Browse files

Fixed DataHandle reference counting implementation:

* AbstractData now stores a weak_ptr to its shared_ptr owning group if existant. This avoids unintentional creation of multiple owning groups where each eventually tries to delete the AbstractData on its own (which will certainly not work)
* Updated DataContainer implementation to use a hash map instead of an unordered map.
parent 231a7c0f
......@@ -416,7 +416,7 @@ namespace campvis {
std::string name = it->first.toStdString();
// copy geometry over to local
_localDataContainer.addData(name + ".geometry", const_cast<GeometryData*>(gd));
_localDataContainer.addDataHandle(name + ".geometry", it->second);
// render
renderGeometryIntoTexture(name);
......
......@@ -25,6 +25,8 @@
#ifndef ABSTRACTDATA_H__
#define ABSTRACTDATA_H__
#include <memory>
#include <tbb/atomic.h>
#include "tgt/bounds.h"
#include "core/coreapi.h"
......@@ -51,12 +53,16 @@ namespace campvis {
// ================================================================================================
class DataHandle;
/**
* Abstract base class for data handled by a DataHandle and stored in a DataContainer.
*
* \todo
*/
class CAMPVIS_CORE_API AbstractData {
friend class DataHandle;
public:
/**
* Constructor, simply calles ReferenceCounted ctor.
......@@ -87,8 +93,11 @@ namespace campvis {
*/
virtual size_t getVideoMemoryFootprint() const = 0;
protected:
private:
/// This weak_ptr points to the shared_ptr owning group of this object, if existant.
/// Should be only accessed by DataHandle (therefore the friendship) in order to avoid
/// multiple owning groups for the same object.
std::weak_ptr<AbstractData> _weakPtr;
};
}
......
......@@ -63,40 +63,30 @@ namespace campvis {
tgtAssert(dh.getData() != 0, "The data in the DataHandle must not be 0!");
tgtAssert(!name.empty(), "The data's name must not be empty.");
{
//tbb::spin_mutex::scoped_lock lock(_localMutex);
tbb::concurrent_unordered_map<std::string, DataHandle>::iterator it = _handles.find(name);
if (it != _handles.end()/* && it->first == name*/) {
it->second = dh;
}
else {
_handles.insert(/*it,*/ std::make_pair(name, dh));
}
}
_handles.erase(name);
_handles.insert(std::make_pair(name, dh));
s_dataAdded(name, dh);
s_changed();
}
bool DataContainer::hasData(const std::string& name) const {
//tbb::spin_mutex::scoped_lock lock(_localMutex);
return (_handles.find(name) != _handles.end());
tbb::concurrent_hash_map<std::string, DataHandle>::const_accessor a;
return _handles.find(a, name);
}
DataHandle DataContainer::getData(const std::string& name) const {
//tbb::spin_mutex::scoped_lock lock(_localMutex);
tbb::concurrent_unordered_map<std::string, DataHandle>::const_iterator it = _handles.find(name);
if (it == _handles.end())
tbb::concurrent_hash_map<std::string, DataHandle>::const_accessor a;
if (_handles.find(a, name)) {
return a->second;
}
else {
return DataHandle(0);
else
return it->second;
}
}
void DataContainer::removeData(const std::string& name) {
tbb::spin_mutex::scoped_lock lock(_localMutex);
tbb::concurrent_unordered_map<std::string, DataHandle>::const_iterator it = _handles.find(name);
if (it != _handles.end()) {
_handles.unsafe_erase(it);
}
_handles.erase(name);
}
std::vector< std::pair< std::string, DataHandle> > DataContainer::getDataHandlesCopy() const {
......@@ -104,20 +94,14 @@ namespace campvis {
toReturn.reserve(_handles.size());
tbb::spin_mutex::scoped_lock lock(_localMutex);
for (tbb::concurrent_unordered_map<std::string, DataHandle>::const_iterator it = _handles.begin(); it != _handles.end(); ++it) {
for (tbb::concurrent_hash_map<std::string, DataHandle>::const_iterator it = _handles.begin(); it != _handles.end(); ++it) {
toReturn.push_back(std::make_pair(it->first, it->second));
}
return toReturn;
}
// tbb::concurrent_unordered_map<std::string, DataHandle> DataContainer::getHandlesCopy() const {
// //tbb::spin_mutex::scoped_lock lock(_localMutex);
// return _handles;
// }
void DataContainer::clear() {
//tbb::spin_mutex::scoped_lock lock(_localMutex);
_handles.clear();
}
......
......@@ -26,7 +26,7 @@
#define DATACONTAINER_H__
#include "sigslot/sigslot.h"
#include <tbb/concurrent_unordered_map.h>
#include <tbb/concurrent_hash_map.h>
#include <tbb/spin_mutex.h>
#include "core/coreapi.h"
......@@ -158,7 +158,7 @@ namespace campvis {
//std::map<std::string, DataHandle> _handles;
mutable tbb::spin_mutex _localMutex;
tbb::concurrent_unordered_map<std::string, DataHandle> _handles;
tbb::concurrent_hash_map<std::string, DataHandle> _handles;
std::string _name;
......
......@@ -29,10 +29,17 @@
namespace campvis {
DataHandle::DataHandle(AbstractData* data)
: _ptr(data)
, _timestamp(clock())
: _timestamp(clock())
{
if (data) {
if (data->_weakPtr.expired()) {
_ptr = std::shared_ptr<AbstractData>(data);
data->_weakPtr = _ptr;
}
else {
_ptr = std::shared_ptr<AbstractData>(data->_weakPtr);
}
}
}
DataHandle::DataHandle(const DataHandle& rhs)
......@@ -43,7 +50,7 @@ namespace campvis {
}
DataHandle& DataHandle::operator=(const DataHandle& rhs) {
if (_ptr != rhs._ptr) {
if (this != &rhs) {
_ptr = rhs._ptr;
_timestamp = rhs._timestamp;
}
......@@ -52,6 +59,7 @@ namespace campvis {
}
DataHandle::~DataHandle() {
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment