Skip to content

Commit

Permalink
STYLE: Declare Observer::m_Event (from itk::Object) as unique_ptr
Browse files Browse the repository at this point in the history
The original code technically allowed an `Observer` (from the implementation
of `itk::Object`) to be copied, causing its destructor to possibly be called
twice, trying to do `delete m_Event` twice on the very same event object. Which
would have undefined behavior, possibly causing a crash.

This commit solves this potential problem by declaring m_Event as a `unique_ptr`,
and removing the user-defined `Observer` destructor (leaving it "implicitly
defaulted", following the Rule of Zero). Thereby it also removes the `virtual`
keyword from the destructor, which is OK, because `Observer` is not a base class.
  • Loading branch information
N-Dekker authored and dzenanz committed Sep 20, 2022
1 parent 5daed92 commit d0a39d3
Showing 1 changed file with 6 additions and 11 deletions.
17 changes: 6 additions & 11 deletions Modules/Core/Common/src/itkObject.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
*=========================================================================*/
#include "itkCommand.h"
#include <algorithm>
#include <memory> // For unique_ptr.

#include "itkSingleton.h"

Expand All @@ -47,16 +48,10 @@ class ITKCommon_HIDDEN Observer
, m_Event(event)
, m_Tag(tag)
{}
virtual ~Observer();
Command::Pointer m_Command;
const EventObject * m_Event;
unsigned long m_Tag;
Command::Pointer m_Command;
std::unique_ptr<const EventObject> m_Event;
unsigned long m_Tag;
};
/* Create Out-of-line Definition */
Observer::~Observer()
{
delete m_Event;
}

class ITKCommon_HIDDEN SubjectImplementation
{
Expand Down Expand Up @@ -248,7 +243,7 @@ SubjectImplementation::HasObserver(const EventObject & event) const
{
for (auto observer : m_Observers)
{
const EventObject * e = observer->m_Event;
const EventObject * e = observer->m_Event.get();
if (e->CheckEvent(&event))
{
return true;
Expand All @@ -267,7 +262,7 @@ SubjectImplementation::PrintObservers(std::ostream & os, Indent indent) const

for (auto observer : m_Observers)
{
const EventObject * e = observer->m_Event;
const EventObject * e = observer->m_Event.get();
const Command * c = observer->m_Command;
os << indent << e->GetEventName() << "(" << c->GetNameOfClass();
if (!c->GetObjectName().empty())
Expand Down

0 comments on commit d0a39d3

Please sign in to comment.