diff --git a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAlignableTransform.h b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAlignableTransform.h
index 009919232450427121dd7dba213bdc1256c77588..3af0c03752dbdce76a62b1778177c19841bb4436 100644
--- a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAlignableTransform.h
+++ b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoAlignableTransform.h
@@ -7,6 +7,7 @@
#include "GeoModelKernel/GeoTransform.h"
#include <vector>
+#include <mutex>
class GeoVAlignmentStore;
@@ -45,6 +46,10 @@ class GeoAlignableTransform final : public GeoTransform
// the memory is unallocated.
GeoTrf::Transform3D* m_delta;
+ // We need to protext m_delta with a mutex in order to avoid
+ // memory corruption in multithreaded applications
+ mutable std::mutex m_deltaMutex;
+
// A list of parents who use this alignable target. They
// must all be notified when the alignment changes!
std::vector<GeoGraphNode*> m_parentList;
diff --git a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoVFullPhysVol.h b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoVFullPhysVol.h
index 795ac30700019a3038eac1e42bf4ea3ed0c4af3f..04c09c66b5d1fb51921e0951b5633426881d585f 100644
--- a/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoVFullPhysVol.h
+++ b/GeoModelCore/GeoModelKernel/GeoModelKernel/GeoVFullPhysVol.h
@@ -7,6 +7,7 @@
#include "GeoModelKernel/GeoVPhysVol.h"
#include "GeoModelKernel/GeoAbsPositionInfo.h"
+#include <mutex>
class GeoVAlignmentStore;
@@ -51,6 +52,11 @@ class GeoVFullPhysVol : public GeoVPhysVol
protected:
virtual ~GeoVFullPhysVol() override;
+ /// Mutex serving dual purpose:
+ /// 1. To protect the absolute position info
+ /// 2. To protect m_daughters container in the derived GeoFullPhysVol
+ mutable std::mutex m_mutex;
+
private:
/// The absolute name of this volume.
std::string m_absName;
diff --git a/GeoModelCore/GeoModelKernel/src/GeoAlignableTransform.cxx b/GeoModelCore/GeoModelKernel/src/GeoAlignableTransform.cxx
index 9542340bbe9d10bde31de88b14d769fd8bf9bb0f..b5c0a0e0b1aeb4ef280d29e0c0ec6ec1c01e6161 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoAlignableTransform.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoAlignableTransform.cxx
@@ -29,20 +29,29 @@ __attribute__ ((flatten))
#endif
GeoTrf::Transform3D GeoAlignableTransform::getTransform(const GeoVAlignmentStore* store) const
{
- const GeoTrf::Transform3D* delta = (store==nullptr ? m_delta : store->getDelta(this));
- return GeoTransform::getTransform(nullptr) * (delta==nullptr ? GeoTrf::Transform3D(GeoTrf::Transform3D::Identity()) : *delta);
+ if(store) {
+ const GeoTrf::Transform3D* delta = store->getDelta(this);
+ return GeoTransform::getTransform(nullptr) * (delta==nullptr ? GeoTrf::Transform3D(GeoTrf::Transform3D::Identity()) : *delta);
+ }
+ else {
+ std::scoped_lock<std::mutex> guard(m_deltaMutex);
+ return GeoTransform::getTransform(nullptr) * (m_delta==nullptr ? GeoTrf::Transform3D(GeoTrf::Transform3D::Identity()) : *m_delta);
+ }
}
void GeoAlignableTransform::setDelta (const GeoTrf::Transform3D& delta, GeoVAlignmentStore* store)
{
if(store==nullptr) {
- if(m_delta && (m_delta->isApprox(delta))) return;
-
- if(m_delta) {
- (*m_delta) = delta;
- }
- else {
- m_delta = new GeoTrf::Transform3D(delta);
+ {
+ std::scoped_lock<std::mutex> guard(m_deltaMutex);
+ if(m_delta && (m_delta->isApprox(delta))) return;
+
+ if(m_delta) {
+ (*m_delta) = delta;
+ }
+ else {
+ m_delta = new GeoTrf::Transform3D(delta);
+ }
}
std::set<GeoGraphNode*> uniqueParents;
@@ -63,9 +72,11 @@ void GeoAlignableTransform::clearDelta(GeoVAlignmentStore* store)
{
// Does not make sence to clear deltas inside Alignment Store
if(store!=nullptr) return;
-
- delete m_delta;
- m_delta = nullptr;
+ {
+ std::scoped_lock<std::mutex> guard(m_deltaMutex);
+ delete m_delta;
+ m_delta = nullptr;
+ }
std::set<GeoGraphNode*> uniqueParents;
for(GeoGraphNode* parent : m_parentList) {
diff --git a/GeoModelCore/GeoModelKernel/src/GeoFullPhysVol.cxx b/GeoModelCore/GeoModelKernel/src/GeoFullPhysVol.cxx
index 9b69ade67c96f5a8cc9ea3e963154b49403bcd3b..0521c9659945cdbcdbd6ac1325c6f6dfdffeebf2 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoFullPhysVol.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoFullPhysVol.cxx
@@ -26,7 +26,7 @@ GeoFullPhysVol::~GeoFullPhysVol()
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);
graphNode->ref();
graphNode->dockTo(this);
@@ -235,15 +235,18 @@ GeoTrf::Transform3D GeoFullPhysVol::getDefX(const GeoVAlignmentStore* store) con
unsigned int GeoFullPhysVol::getNChildNodes() const
{
+ std::scoped_lock<std::mutex> guard(m_mutex);
return m_daughters.size();
}
const GeoGraphNode * const * GeoFullPhysVol::getChildNode(unsigned int i) const
{
+ std::scoped_lock<std::mutex> guard(m_mutex);
return &(m_daughters[i]);
}
const GeoGraphNode * const * GeoFullPhysVol::findChildNode(const GeoGraphNode * n) const
{
+ std::scoped_lock<std::mutex> guard(m_mutex);
std::vector<const GeoGraphNode *>::const_iterator i = std::find(m_daughters.begin(),m_daughters.end(),n);
if (i==m_daughters.end()) {
return nullptr;
diff --git a/GeoModelCore/GeoModelKernel/src/GeoPhysVol.cxx b/GeoModelCore/GeoModelKernel/src/GeoPhysVol.cxx
index cbc44e3ef1a362d4b60373e5bc36572cf6d787db..7b6161be085854086be3704e9a8d9981129714aa 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoPhysVol.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoPhysVol.cxx
@@ -19,13 +19,13 @@ GeoPhysVol::GeoPhysVol(const GeoLogVol* LogVol)
GeoPhysVol::~GeoPhysVol()
{
- std::lock_guard<std::mutex> lk(m_muxVec);
+ std::scoped_lock<std::mutex> lk(m_muxVec);
for(const GeoGraphNode* daughter : m_daughters) daughter->unref();
}
void GeoPhysVol::add(GeoGraphNode* graphNode)
{
- std::lock_guard<std::mutex> lk(m_muxVec);
+ std::scoped_lock<std::mutex> lk(m_muxVec);
m_daughters.push_back(graphNode);
graphNode->ref();
graphNode->dockTo(this);
@@ -88,7 +88,7 @@ void GeoPhysVol::exec(GeoNodeAction *action) const
}
else {
// FIXME: m_daughters access is now protected in other methods, but having the lock here makes a deadlock
- // std::lock_guard<std::mutex> lk(m_muxVec);
+ // std::scoped_lock<std::mutex> lk(m_muxVec);
// TODO: Think more thouroughly about thread-safe of this class...!!
for(size_t c = 0; c < m_daughters.size (); c++) {
m_daughters[c]->exec(action);
@@ -195,19 +195,19 @@ GeoTrf::Transform3D GeoPhysVol::getDefX(const GeoVAlignmentStore* store) const {
unsigned int GeoPhysVol::getNChildNodes() const
{
- std::lock_guard<std::mutex> lk(m_muxVec);
+ std::scoped_lock<std::mutex> lk(m_muxVec);
return m_daughters.size();
}
const GeoGraphNode * const * GeoPhysVol::getChildNode(unsigned int i) const
{
- std::lock_guard<std::mutex> lk(m_muxVec);
+ std::scoped_lock<std::mutex> lk(m_muxVec);
return &(m_daughters[i]);
}
const GeoGraphNode * const * GeoPhysVol::findChildNode(const GeoGraphNode * n) const
{
- std::lock_guard<std::mutex> lk(m_muxVec);
+ std::scoped_lock<std::mutex> lk(m_muxVec);
std::vector<const GeoGraphNode *>::const_iterator i = std::find(m_daughters.begin(),m_daughters.end(),n);
if (i==m_daughters.end()) {
return nullptr;
diff --git a/GeoModelCore/GeoModelKernel/src/GeoVFullPhysVol.cxx b/GeoModelCore/GeoModelKernel/src/GeoVFullPhysVol.cxx
index 6555d46344900f5b0f492c587e0746342648ff53..598484f7970f04c462653a3912e3997e977ebb86 100755
--- a/GeoModelCore/GeoModelKernel/src/GeoVFullPhysVol.cxx
+++ b/GeoModelCore/GeoModelKernel/src/GeoVFullPhysVol.cxx
@@ -30,6 +30,8 @@ const GeoTrf::Transform3D & GeoVFullPhysVol::getAbsoluteTransform(GeoVAlignmentS
//------------------------------------------------------------------------------------------------//
if(isShared()) throw std::runtime_error(errorMessage);
+ std::scoped_lock<std::mutex> guard(m_mutex);
+
if(store==nullptr && !m_absPosInfo) m_absPosInfo = new GeoAbsPositionInfo();
//
@@ -78,6 +80,7 @@ const GeoTrf::Transform3D & GeoVFullPhysVol::getAbsoluteTransform(GeoVAlignmentS
const GeoTrf::Transform3D& GeoVFullPhysVol::getCachedAbsoluteTransform(const GeoVAlignmentStore* store) const
{
if(store==nullptr) {
+ std::scoped_lock<std::mutex> guard(m_mutex);
if(m_absPosInfo->getAbsTransform()) return *m_absPosInfo->getAbsTransform();
}
else {
@@ -90,6 +93,7 @@ const GeoTrf::Transform3D& GeoVFullPhysVol::getCachedAbsoluteTransform(const Geo
void GeoVFullPhysVol::clearPositionInfo() const
{
+ std::scoped_lock<std::mutex> guard(m_mutex);
delete m_absPosInfo;
m_absPosInfo = nullptr;
}
@@ -105,6 +109,8 @@ const GeoTrf::Transform3D& GeoVFullPhysVol::getDefAbsoluteTransform(GeoVAlignmen
//------------------------------------------------------------------------------------------------//
if(isShared()) throw std::runtime_error(errorMessage);
+ std::scoped_lock<std::mutex> guard(m_mutex);
+
if(store==nullptr && !m_absPosInfo) m_absPosInfo = new GeoAbsPositionInfo();
//
@@ -153,6 +159,7 @@ const GeoTrf::Transform3D& GeoVFullPhysVol::getDefAbsoluteTransform(GeoVAlignmen
const GeoTrf::Transform3D& GeoVFullPhysVol::getCachedDefAbsoluteTransform(const GeoVAlignmentStore* store) const
{
if(store==nullptr) {
+ std::scoped_lock<std::mutex> guard(m_mutex);
if(m_absPosInfo->getDefAbsTransform()) return *m_absPosInfo->getDefAbsTransform();
}
else {