#6437 Fix Signal crash and improve signal code

* The crash fix is ensuring we don't loop through m_disconnectCallbacks
  in DeleteSignal::send()
* This is because the callback will end up calling disconnect
  which in turn erases the entry from m_disconnectCallbacks,
  invalidating the iterators used in the loop.
This commit is contained in:
Gaute Lindkvist
2020-09-04 13:57:51 +02:00
parent baaa18bdd5
commit d08444ec77
4 changed files with 109 additions and 17 deletions

View File

@@ -50,6 +50,7 @@ set( PROJECT_FILES
cafNotificationCenter.h
cafSignal.h
cafSignal.cpp
cafTristate.cpp
cafTristate.h

View File

@@ -119,6 +119,8 @@ void PdmObjectHandle::objectsWithReferringPtrFields( std::vector<PdmObjectHandle
//--------------------------------------------------------------------------------------------------
void PdmObjectHandle::prepareForDelete()
{
this->sendDeleteSignal();
m_parentField = nullptr;
for ( size_t i = 0; i < m_capabilities.size(); ++i )

View File

@@ -0,0 +1,96 @@
//##################################################################################################
//
// Custom Visualization Core library
// Copyright (C) 2020- Ceetron Solutions AS
//
// This library may be used under the terms of either the GNU General Public License or
// the GNU Lesser General Public License as follows:
//
// GNU General Public License Usage
// This library is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This library is distributed in the hope that it will be useful, but WITHOUT ANY
// WARRANTY; without even the implied warranty of MERCHANTABILITY or
// FITNESS FOR A PARTICULAR PURPOSE.
//
// See the GNU General Public License at <<http://www.gnu.org/licenses/gpl.html>>
// for more details.
//
// GNU Lesser General Public License Usage
// This library is free software; you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation; either version 2.1 of the License, or
// (at your option) any later version.
//
// This library is distributed in the hope that it will be useful, but WITHOUT ANY
// WARRANTY; without even the implied warranty of MERCHANTABILITY or
// FITNESS FOR A PARTICULAR PURPOSE.
//
// See the GNU Lesser General Public License at <<http://www.gnu.org/licenses/lgpl-2.1.html>>
// for more details.
//
//##################################################################################################
#include "cafSignal.h"
using namespace caf;
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
DeleteSignal::DeleteSignal( SignalObserver* observer )
: m_observer( observer )
{
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
void DeleteSignal::disconnect( SignalObserver* observer )
{
// Remove the observer from the call back list
m_disconnectCallbacks.erase( observer );
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
void DeleteSignal::send()
{
// Make sure we swap out the disconnect signals to avoid
// erasing from the signal-map while looping through it.
std::map<SignalObserver*, DisconnectCallback> disconnectCallbacks;
disconnectCallbacks.swap( m_disconnectCallbacks );
for ( auto signalCallbackPair : disconnectCallbacks )
{
signalCallbackPair.second( m_observer );
}
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
SignalObserver::SignalObserver()
: beingDeleted( this )
{
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
void SignalObserver::sendDeleteSignal()
{
// Make sure we
beingDeleted.send();
}
//--------------------------------------------------------------------------------------------------
///
//--------------------------------------------------------------------------------------------------
SignalObserver::~SignalObserver()
{
sendDeleteSignal();
}

View File

@@ -56,10 +56,7 @@ class DeleteSignal
public:
using DisconnectCallback = std::function<void( SignalObserver* )>;
DeleteSignal( SignalObserver* observer )
: m_observer( observer )
{
}
DeleteSignal( SignalObserver* observer );
template <typename ClassType>
void connect( ClassType* signal )
@@ -72,14 +69,8 @@ public:
if ( signalCasted ) m_disconnectCallbacks[signalCasted] = disconnectCallback;
}
void disconnect( SignalObserver* observer ) { m_disconnectCallbacks.erase( observer ); }
void send()
{
for ( auto signalCallbackPair : m_disconnectCallbacks )
{
signalCallbackPair.second( m_observer );
}
}
void disconnect( SignalObserver* observer );
void send();
private:
std::map<SignalObserver*, DisconnectCallback> m_disconnectCallbacks;
@@ -107,11 +98,9 @@ public:
DeleteSignal beingDeleted;
public:
SignalObserver()
: beingDeleted( this )
{
}
virtual ~SignalObserver() { beingDeleted.send(); }
SignalObserver();
void sendDeleteSignal();
virtual ~SignalObserver();
}; // namespace caf
@@ -170,6 +159,7 @@ public:
};
connect( observer, lambda );
}
template <typename ClassType>
void connect( ClassType* observer, const MemberCallback& callback )
{
@@ -187,6 +177,7 @@ public:
m_observerCallbacks.erase( observer );
observer->beingDeleted.disconnect( this );
}
void send( Args... args )
{
for ( auto observerCallbackPair : m_observerCallbacks )
@@ -197,12 +188,14 @@ public:
}
}
}
void block( SignalObserver* observer )
{
auto it = m_observerCallbacks.find( observer );
CAF_ASSERT( it != m_observerCallbacks.end() );
it->second.second = false;
}
void unblock( SignalObserver* observer )
{
auto it = m_observerCallbacks.find( observer );