From c82064f87c72fcafb5b6326167852684465c1e90 Mon Sep 17 00:00:00 2001 From: Giuseppe Carboni Date: Thu, 1 Aug 2019 14:29:49 +0200 Subject: [PATCH] Fix #431, dropped the use of the SecureArea in the AntennaBoss WatchingThread (#432) * Fix #431, dropped the use of the SecureArea in the AntennaBoss WatchingThread The AntennaBoss WatchingThread is now free to run without being blocked by a SecureArea. This allows both the WatchingThread and the WorkingThread to be executed in parallel. This was tested using the SRT ACU simulator. The test has been running for more than one hour without issues. Further testing with the SRT will be performed soon (2019/07/25). This SHOULD NOT break the MedicinaMount, but it was not tested, so I'm not 100% sure. P.S.: I also tried a setup with both threads not protected by a SecureArea, but after a while the tracking behaved unexpectedly, sending to the ACU a set of coordinates with an elevation of more than 2000 degrees. This behavior was most likely caused by the WorkingThread not protected by the SecureArea. * Fix #431, improved synchronization between AntennaBoss threads The load/unloadMount, load/unloadRefraction load/unloadPointingModel methods are now thread safe in order to minimize (unfortunately not avoid completely) concurrency errors between the AntennaBoss WatchingThread and WorkingThread The TimeTaggedCircularArray object is now thread safe. --- .../IRALibrary/include/TimeTaggedCircularArray.h | 2 ++ .../IRALibrary/src/TimeTaggedCircularArray.cpp | 15 +++++++++++++-- Common/Servers/AntennaBoss/include/BossCore.h | 4 ++++ .../Servers/AntennaBoss/include/WatchingThread.h | 4 ++-- .../Servers/AntennaBoss/src/AntennaBossImpl.cpp | 4 ++-- Common/Servers/AntennaBoss/src/BossCore.cpp | 6 ++++++ Common/Servers/AntennaBoss/src/WatchingThread.cpp | 12 +++++------- 7 files changed, 34 insertions(+), 13 deletions(-) diff --git a/Common/Libraries/IRALibrary/include/TimeTaggedCircularArray.h b/Common/Libraries/IRALibrary/include/TimeTaggedCircularArray.h index 2ce17ad34..fa1fd0f00 100644 --- a/Common/Libraries/IRALibrary/include/TimeTaggedCircularArray.h +++ b/Common/Libraries/IRALibrary/include/TimeTaggedCircularArray.h @@ -12,6 +12,7 @@ /* Andrea Orlati(aorlati@ira.inaf.it) 31/01/2008 changed the algorithm of selectPoint() */ /* Andrea Orlati(aorlati@ira.inaf.it) 31/01/2008 fixed a bug in selectPoint() that causes problem when longitude crosses the 360 */ /* Andrea Orlati(aorlati@ira.inaf.it) 08/09/2010 added the averagePoint method */ +/* G. Carboni(giuseppe.carboni@inaf.it) 26/07/2019 the class is now thread safe */ #include #include "Definitions.h" @@ -161,6 +162,7 @@ protected: private: CTimeTaggedCircularArray(const CTimeTaggedCircularArray&); const CTimeTaggedCircularArray& operator =(const CTimeTaggedCircularArray& src); + mutable BACIMutex m_mutex; }; } diff --git a/Common/Libraries/IRALibrary/src/TimeTaggedCircularArray.cpp b/Common/Libraries/IRALibrary/src/TimeTaggedCircularArray.cpp index 73c37fdc1..e8dd6518b 100644 --- a/Common/Libraries/IRALibrary/src/TimeTaggedCircularArray.cpp +++ b/Common/Libraries/IRALibrary/src/TimeTaggedCircularArray.cpp @@ -25,11 +25,13 @@ CTimeTaggedCircularArray::~CTimeTaggedCircularArray() void CTimeTaggedCircularArray::empty() { + baci::ThreadSyncGuard guard(&m_mutex); m_head=m_free=0; } bool CTimeTaggedCircularArray::addPoint(const double& azimuth,const double& elevation,const TIMEVALUE& time) -{ +{ + baci::ThreadSyncGuard guard(&m_mutex); TIMEVALUE *tmp; if (isEmpty()) { } @@ -47,6 +49,7 @@ bool CTimeTaggedCircularArray::addPoint(const double& azimuth,const double& elev bool CTimeTaggedCircularArray::getPoint(unsigned pos,double& azimuth,double& elevation,TIMEVALUE& time) const { + baci::ThreadSyncGuard guard(&m_mutex); unsigned ss,pp; if (isEmpty()) return false; ss=elements(); @@ -60,21 +63,25 @@ bool CTimeTaggedCircularArray::getPoint(unsigned pos,double& azimuth,double& ele const double& CTimeTaggedCircularArray::getLastAzimuth() const { + baci::ThreadSyncGuard guard(&m_mutex); return m_lastAzimuth; } -const double& CTimeTaggedCircularArray::getLastElevation() const +const double& CTimeTaggedCircularArray::getLastElevation() const { + baci::ThreadSyncGuard guard(&m_mutex); return m_lastElevation; } const TIMEVALUE& CTimeTaggedCircularArray::getLastTime() const { + baci::ThreadSyncGuard guard(&m_mutex); return m_lastTime; } unsigned CTimeTaggedCircularArray::elements() const { + baci::ThreadSyncGuard guard(&m_mutex); if (isEmpty()) return 0; if (m_head>m_free) return (m_size-m_head)+m_free; else return m_free-m_head; @@ -82,6 +89,7 @@ unsigned CTimeTaggedCircularArray::elements() const void CTimeTaggedCircularArray::purge(const TIMEVALUE& time) { + baci::ThreadSyncGuard guard(&m_mutex); unsigned ss=elements(); unsigned pp=0; for (unsigned i=0;i *param, + CWatchingThread(const ACE_CString& name,CBossCore *param, const ACS::TimeInterval& responseTime=ThreadBase::defaultResponseTime,const ACS::TimeInterval& sleepTime=ThreadBase::defaultSleepTime); /** @@ -51,7 +51,7 @@ public: */ virtual void runLoop(); private: - IRA::CSecureArea *m_core; + CBossCore *boss; }; #endif /*WATCHINGTHREAD_H_*/ diff --git a/Common/Servers/AntennaBoss/src/AntennaBossImpl.cpp b/Common/Servers/AntennaBoss/src/AntennaBossImpl.cpp index 6250cbdf5..f40a15fd0 100644 --- a/Common/Servers/AntennaBoss/src/AntennaBossImpl.cpp +++ b/Common/Servers/AntennaBoss/src/AntennaBossImpl.cpp @@ -151,8 +151,8 @@ void AntennaBossImpl::initialize() throw (ACSErr::ACSbaseExImpl) } boss->initialize(); //could throw (ComponentErrors::UnexpectedExImpl) try { - m_watchingThread=getContainerServices()->getThreadManager()->create *> - ("BOSSWATCHER",m_core); + m_watchingThread=getContainerServices()->getThreadManager()->create + ("BOSSWATCHER",boss); m_workingThread=getContainerServices()->getThreadManager()->create *> ("BOSSWORKER",m_core); } diff --git a/Common/Servers/AntennaBoss/src/BossCore.cpp b/Common/Servers/AntennaBoss/src/BossCore.cpp index 29d86bb70..b0a916559 100644 --- a/Common/Servers/AntennaBoss/src/BossCore.cpp +++ b/Common/Servers/AntennaBoss/src/BossCore.cpp @@ -1393,6 +1393,7 @@ void CBossCore::changeBossStatus(const Management::TSystemStatus& status) void CBossCore::loadMount(Antenna::Mount_var& ref,bool& errorDetected) const throw (ComponentErrors::CouldntGetComponentExImpl) { + ThreadSyncGuard guard(&m_mountMutex); // We take the mutex in order to avoid concurrency errors between the WatchingThread and the WorkingThread if ((ref!=Antenna::Mount::_nil()) && (errorDetected)) { // if reference was already taken, but an error was found....dispose the reference try { m_services->releaseComponent((const char*)ref->name()); @@ -1423,6 +1424,7 @@ void CBossCore::loadMount(Antenna::Mount_var& ref,bool& errorDetected) const thr void CBossCore::unloadMount(Antenna::Mount_var& ref) const { + ThreadSyncGuard guard(&m_mountMutex); // We take the mutex in order to avoid concurrency errors between the WatchingThread and the WorkingThread if (!CORBA::is_nil(ref)) { try { m_services->releaseComponent((const char*)ref->name()); @@ -1442,6 +1444,7 @@ void CBossCore::unloadMount(Antenna::Mount_var& ref) const void CBossCore::loadPointingModel(Antenna::PointingModel_var& ref) const throw (ComponentErrors::CouldntGetComponentExImpl) { + ThreadSyncGuard guard(&m_pmMutex); // We take the mutex in order to avoid concurrency errors between the WatchingThread and the WorkingThread if (CORBA::is_nil(ref)) { //only if it has not been retrieved yet try { ref=m_services->getComponent((const char*)m_config->getPointingModelInstance()); @@ -1457,6 +1460,7 @@ void CBossCore::loadPointingModel(Antenna::PointingModel_var& ref) const throw ( void CBossCore::loadRefraction(Antenna::Refraction_var& ref) const throw (ComponentErrors::CouldntGetComponentExImpl) { + ThreadSyncGuard guard(&m_refractionMutex); // We take the mutex in order to avoid concurrency errors between the WatchingThread and the WorkingThread if (CORBA::is_nil(ref)) { //only if it has not been retrieved yet try { ref=m_services->getComponent((const char*)m_config->getRefractionInstance()); @@ -1472,6 +1476,7 @@ void CBossCore::loadRefraction(Antenna::Refraction_var& ref) const throw (Compon void CBossCore::unloadPointingModel(Antenna::PointingModel_var& ref) const { + ThreadSyncGuard guard(&m_pmMutex); // We take the mutex in order to avoid concurrency errors between the WatchingThread and the WorkingThread if (!CORBA::is_nil(ref)) { try { m_services->releaseComponent((const char*)ref->name()); @@ -1487,6 +1492,7 @@ void CBossCore::unloadPointingModel(Antenna::PointingModel_var& ref) const void CBossCore::unloadRefraction(Antenna::Refraction_var& ref) const { + ThreadSyncGuard guard(&m_refractionMutex); // We take the mutex in order to avoid concurrency errors between the WatchingThread and the WorkingThread if (!CORBA::is_nil(ref)) { try { m_services->releaseComponent((const char*)ref->name()); diff --git a/Common/Servers/AntennaBoss/src/WatchingThread.cpp b/Common/Servers/AntennaBoss/src/WatchingThread.cpp index 07f112b72..8829b081f 100644 --- a/Common/Servers/AntennaBoss/src/WatchingThread.cpp +++ b/Common/Servers/AntennaBoss/src/WatchingThread.cpp @@ -5,8 +5,8 @@ _IRA_LOGFILTER_IMPORT; -CWatchingThread::CWatchingThread(const ACE_CString& name,IRA::CSecureArea *param, - const ACS::TimeInterval& responseTime,const ACS::TimeInterval& sleepTime) : ACS::Thread(name,responseTime,sleepTime), m_core(param) +CWatchingThread::CWatchingThread(const ACE_CString& name,CBossCore *param, + const ACS::TimeInterval& responseTime,const ACS::TimeInterval& sleepTime) : ACS::Thread(name,responseTime,sleepTime), boss(param) { AUTO_TRACE("CWatchingThread::CWatchingThread()"); } @@ -28,10 +28,8 @@ void CWatchingThread::onStart() void CWatchingThread::runLoop() { - IRA::CSecAreaResourceWrapper resource=m_core->Get("WATCHINGTHREAD:runLoop"); try { - //printf("updateAttributes\n"); - if(!resource->updateAttributes()) return; + if(!boss->updateAttributes()) return; } catch (ACSErr::ACSbaseExImpl& E) { _ADD_BACKTRACE(ComponentErrors::WatchDogErrorExImpl,_dummy,E,"CWatchingThread::runLoop()"); @@ -39,7 +37,7 @@ void CWatchingThread::onStart() _IRA_LOGFILTER_LOG_EXCEPTION(_dummy,LM_ERROR); } try { - resource->checkStatus(); + boss->checkStatus(); } catch (ACSErr::ACSbaseExImpl& E) { _ADD_BACKTRACE(ComponentErrors::WatchDogErrorExImpl,_dummy,E,"CWatchingThread::runLoop()"); @@ -47,7 +45,7 @@ void CWatchingThread::onStart() _IRA_LOGFILTER_LOG_EXCEPTION(_dummy,LM_ERROR); } try { - resource->publishData(); + boss->publishData(); } catch (ACSErr::ACSbaseExImpl& E) { _ADD_BACKTRACE(ComponentErrors::WatchDogErrorExImpl,_dummy,E,"CWatchingThread::runLoop()"); -- GitLab