Skip to content
Snippets Groups Projects
Commit ec362447 authored by Johannes Junggeburth's avatar Johannes Junggeburth :dog2:
Browse files

Fix random pointer deletion

parent 9c8700f2
Branches
Tags
1 merge request!232Fix random pointer deletion
Pipeline #6601740 passed
......@@ -4,49 +4,70 @@
#ifndef GEOMODELKERNEL_GeoIntrusivePtr_H
#define GEOMODELKERNEL_GeoIntrusivePtr_H
#include <GeoModelKernel/RCBase.h>
#include <type_traits>
template<typename GeoType>
class GeoIntrusivePtr{
template<typename GeoType>
class GeoIntrusivePtr {
public:
template <typename GeoTypeGrp> friend class GeoIntrusivePtr;
GeoIntrusivePtr() = default;
explicit GeoIntrusivePtr() noexcept = default;
// Standard constructor taking a bare pointer
GeoIntrusivePtr(GeoType* obj):
GeoIntrusivePtr(GeoType* obj) noexcept:
m_ptr{obj} {
if (m_ptr) obj->ref();
}
/// Copy constructor
template <class GeoTypeGrp>
GeoIntrusivePtr(const GeoIntrusivePtr<GeoTypeGrp>& other):
GeoIntrusivePtr(const GeoIntrusivePtr& other) noexcept:
GeoIntrusivePtr{other.get()} {}
/// Copy constructor for derived types
template <typename GeoTypeGrp,
typename = typename std::enable_if<!std::is_same<GeoType,GeoTypeGrp>::value, bool>>
GeoIntrusivePtr(const GeoIntrusivePtr<GeoTypeGrp>& other) noexcept:
GeoIntrusivePtr{other.get()} {}
/// Move constructor
explicit GeoIntrusivePtr(GeoIntrusivePtr&& other) noexcept:
m_ptr{other.m_ptr} {
other.m_ptr = nullptr;
}
/// Move constructor for derived types
template <typename GeoTypeGrp,
typename = typename std::enable_if<!std::is_same<GeoType,GeoTypeGrp>::value, bool>>
GeoIntrusivePtr(GeoIntrusivePtr<GeoTypeGrp>&& other) noexcept:
m_ptr{other.m_ptr} {
other.m_ptr = nullptr;
}
/// Assignment operator
template <class GeoTypeGrp>
GeoIntrusivePtr& operator=(const GeoIntrusivePtr<GeoTypeGrp>& other) {
if (m_ptr != other.get()) {
if(m_ptr) m_ptr->unref();
m_ptr = other.get();
if(m_ptr) m_ptr->ref();
}
GeoIntrusivePtr& operator=(const GeoIntrusivePtr& other) noexcept {
reset(other.get());
return *this;
}
GeoIntrusivePtr& operator=(GeoType* other) noexcept {
reset(other);
return *this;
}
/// Move constructor
template <class GeoTypeGrp>
GeoIntrusivePtr(GeoIntrusivePtr<GeoTypeGrp>&& other):
m_ptr{std::move(other.m_ptr)} { other.m_ptr = nullptr;}
/// Move assignment operator
template <class GeoTypeGrp>
GeoIntrusivePtr& operator=(GeoIntrusivePtr<GeoTypeGrp>&& other) {
if (m_ptr != other.get()) {
if(m_ptr) m_ptr->unref();
m_ptr = std::move(other.m_ptr);
} else if (m_ptr) {
GeoIntrusivePtr& operator=(GeoIntrusivePtr&& other) {
if (m_ptr && m_ptr == other.get()) {
m_ptr->unref();
} else {
reset(other.get());
}
other.m_ptr = nullptr;
return *this;
}
/// Reset the pointer
void reset(GeoType* ptr = nullptr) {
if (m_ptr == ptr) return;
if (m_ptr) m_ptr->unref();
m_ptr = ptr;
if (m_ptr) m_ptr->ref();
}
/// Destructor
~GeoIntrusivePtr() {
if (m_ptr) m_ptr->unref();
......@@ -66,10 +87,10 @@ class GeoIntrusivePtr{
/// Invalidity operator
bool operator!() const { return !m_ptr; }
/// Comparison operator
template <class GeoTypeGrp>
bool operator==(const GeoIntrusivePtr<GeoTypeGrp>& other) const {
return m_ptr == other.m_ptr;
}
// template <class GeoTypeGrp>
// bool operator==(const GeoIntrusivePtr<GeoTypeGrp>& other) const {
// return m_ptr == other.m_ptr;
// }
bool operator==(GeoType* other) const {
return m_ptr == other;
}
......
......@@ -57,7 +57,7 @@ class GeoSerialTransformer : public GeoGraphNode
std::unique_ptr<const GeoXF::Function> m_function{};
// The physical volume to be multiply placed.
GeoIntrusivePtr<const GeoVPhysVol>m_physVol{};
GeoIntrusivePtr<const GeoVPhysVol> m_physVol{};
};
#endif
......@@ -6,12 +6,6 @@
#define GEOMODELKERNEL_GEOVPHYSVOL_H
#include "GeoModelKernel/GeoIntrusivePtr.h"
class GeoVPhysVol;
using PVLink = GeoIntrusivePtr<GeoVPhysVol>;
using GeoPVLink = PVLink;
using PVConstLink = GeoIntrusivePtr<const GeoVPhysVol>;
using GeoPVConstLink = PVConstLink;
#include "GeoModelKernel/GeoDefinitions.h"
#include "GeoModelKernel/Query.h"
#include <string>
......@@ -22,10 +16,12 @@ using GeoPVConstLink = PVConstLink;
class GeoVolumeAction;
class GeoVAlignmentStore;
class GeoVPhysVol : public GeoGraphNode
{
class GeoVPhysVol : public GeoGraphNode {
public:
GeoVPhysVol(const GeoLogVol* LogVol);
using PVLink = GeoIntrusivePtr<GeoVPhysVol>;
using PVConstLink = GeoIntrusivePtr<const GeoVPhysVol>;
GeoVPhysVol(const GeoLogVol* LogVol);
/// Returns true if the physical volume is accessed by more than one parent.
......@@ -102,5 +98,10 @@ class GeoVPhysVol : public GeoGraphNode
GeoIntrusivePtr<const GeoLogVol> m_logVol{};
};
using PVLink = GeoVPhysVol::PVLink;
using PVConstLink = GeoVPhysVol::PVConstLink;
using GeoPVLink = PVLink;
using GeoPVConstLink = PVConstLink;
#endif
......@@ -26,18 +26,20 @@ class RCBase {
RCBase() = default;
// Increase the reference count
void ref() const {
void ref() const noexcept {
++m_count;
}
// Decreases the reference count. When the reference count
// falls to zero, the object deletes itself.
void unref () const {
if (--m_count == 0) delete this;
void unref () const noexcept{
if (--m_count == 0) {
delete this;
}
}
// Return the reference count.
unsigned int refCount () const {
unsigned int refCount () const noexcept {
return m_count.load();
}
......
......@@ -22,7 +22,8 @@ void GeoFullPhysVol::add(GeoGraphNode* graphNode)
{
if(m_cloneOrigin) throw std::runtime_error("Attempt to modify contents of a cloned FPV");
std::scoped_lock<std::mutex> guard(m_mutex);
m_daughters.push_back(graphNode);
GeoIntrusivePtr<GeoGraphNode> nodePtr{graphNode};
m_daughters.emplace_back(nodePtr);
graphNode->dockTo(this);
}
......
......@@ -13,15 +13,13 @@
GeoPhysVol::GeoPhysVol(const GeoLogVol* LogVol)
: GeoVPhysVol(LogVol)
{
}
: GeoVPhysVol(LogVol) {}
void GeoPhysVol::add(GeoGraphNode* graphNode)
{
void GeoPhysVol::add(GeoGraphNode* graphNode) {
std::scoped_lock<std::mutex> lk(m_muxVec);
m_daughters.push_back(graphNode);
GeoIntrusivePtr<GeoGraphNode> nodePtr{graphNode};
m_daughters.emplace_back(nodePtr);
graphNode->dockTo(this);
}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment