STYLE: Declare Observer::m_Event (from itk::Object) as unique_ptr

N-Dekker · dzenanz · commit d0a39d3dda8f · 2022-09-20T08:33:00.000-04:00
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.
diff --git a/Modules/Core/Common/src/itkObject.cxx b/Modules/Core/Common/src/itkObject.cxx
@@ -27,6 +27,7 @@
  *=========================================================================*/
 #include "itkCommand.h"
 #include <algorithm>
+#include <memory> // For unique_ptr.
 
 #include "itkSingleton.h"
 
@@ -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
 {
@@ -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;
@@ -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())
