signal_impl: Trying to do the ref-counting with std::shared_ptr.

murraycu · murraycu · commit 46b9dff3274c · 2016-04-23T21:29:42.000+02:00
Bug #764935
diff --git a/sigc++/signal.h b/sigc++/signal.h
@@ -255,7 +255,7 @@ struct signal_emit
    * @param a Arguments to be passed on to the slots.
    * @return The accumulated return values of the slot invocations as processed by the accumulator.
    */
-  static decltype(auto) emit(signal_impl* impl, type_trait_take_t<T_arg>... a)
+  static decltype(auto) emit(const std::shared_ptr<internal::signal_impl>& impl, type_trait_take_t<T_arg>... a)
   {
     using slot_iterator_buf_type = internal::slot_iterator_buf<self_type, T_return>;
 
@@ -306,7 +306,7 @@ struct signal_emit<T_return, void, T_arg...>
    * @param a Arguments to be passed on to the slots.
    * @return The return value of the last slot invoked.
    */
-  static decltype(auto) emit(signal_impl* impl, type_trait_take_t<T_arg>... a)
+  static decltype(auto) emit(const std::shared_ptr<internal::signal_impl>& impl, type_trait_take_t<T_arg>... a)
   {
     if (!impl || impl->slots_.empty())
       return T_return();
@@ -363,7 +363,7 @@ struct signal_emit<void, void, T_arg...>
    * passed directly on to the slots.
    * @param a Arguments to be passed on to the slots.
    */
-  static decltype(auto) emit(signal_impl* impl, type_trait_take_t<T_arg>... a)
+  static decltype(auto) emit(const std::shared_ptr<internal::signal_impl>& impl, type_trait_take_t<T_arg>... a)
   {
     if (!impl || impl->slots_.empty())
       return;
diff --git a/sigc++/signal_base.cc b/sigc++/signal_base.cc
@@ -28,27 +28,20 @@ namespace internal
 // when the slot is disconnected. Bug 167714.
 struct self_and_iter : public notifiable
 {
-  signal_impl* self_;
+  const std::shared_ptr<signal_impl> self_;
   const signal_impl::iterator_type iter_;
 
-  self_and_iter(signal_impl* self, const signal_impl::iterator_type& iter) : self_(self), iter_(iter) {}
+  self_and_iter(const std::shared_ptr<signal_impl>& self, const signal_impl::iterator_type& iter) : self_(self), iter_(iter) {}
 };
 
-signal_impl::signal_impl() : ref_count_(0), exec_count_(0), deferred_(false)
+signal_impl::signal_impl() : exec_count_(0), deferred_(false)
 {
 }
 
 signal_impl::~signal_impl()
 {
-  // unreference() must not call ~signal_impl() while clear() is executing.
-  ++ref_count_;
-
   // Disconnect all slots before *this is deleted.
   clear();
-
-  // Now ref_count_ can be cleared again (not really necessary), but don't do it
-  // with a call to unreference(). That would invoke ~signal_impl() recursively.
-  --ref_count_;
 }
 
 // only MSVC needs this to guarantee that all new/delete are executed from the DLL module
@@ -72,7 +65,7 @@ signal_impl::clear()
   // Don't let signal_impl::notify() erase the slots. It would invalidate the
   // iterator in the following loop.
   const bool saved_deferred = deferred_;
-  signal_exec exec(this);
+  signal_exec exec(shared_from_this());
 
   // Disconnect all connected slots before they are deleted.
   // signal_impl::notify() will be called and delete the self_and_iter structs.
@@ -128,7 +121,7 @@ signal_impl::erase(iterator_type i)
   // Don't let signal_impl::notify() erase the slot. It would be more
   // difficult to get the correct return value from signal_impl::erase().
   const bool saved_deferred = deferred_;
-  signal_exec exec(this);
+  signal_exec exec(shared_from_this());
 
   // Disconnect the slot before it is deleted.
   // signal_impl::notify() will be called and delete the self_and_iter struct.
@@ -142,7 +135,7 @@ signal_impl::erase(iterator_type i)
 void
 signal_impl::add_notification_to_iter(const signal_impl::iterator_type& iter)
 {
-  auto si = new self_and_iter(this, iter);
+  auto si = new self_and_iter(shared_from_this(), iter);
   iter->set_parent(si, &signal_impl::notify_self_and_iter_of_invalidated_slot);
 }
 
@@ -168,7 +161,7 @@ signal_impl::sweep()
   // The deletion of a slot may cause the deletion of a signal_base,
   // a decrementation of ref_count_, and the deletion of this.
   // In that case, the deletion of this is deferred to ~signal_exec().
-  signal_exec exec(this);
+  signal_exec exec(shared_from_this());
 
   deferred_ = false;
   auto i = slots_.begin();
@@ -207,13 +200,12 @@ signal_impl::notify_self_and_iter_of_invalidated_slot(notifiable* d)
 
 } /* namespace internal */
 
-signal_base::signal_base() noexcept : impl_(nullptr)
+signal_base::signal_base() noexcept
 {
 }
 
 signal_base::signal_base(const signal_base& src) noexcept : trackable(), impl_(src.impl())
 {
-  impl_->reference();
 }
 
 signal_base::signal_base(signal_base&& src) : trackable(std::move(src)), impl_(std::move(src.impl_))
@@ -223,10 +215,6 @@ signal_base::signal_base(signal_base&& src) : trackable(std::move(src)), impl_(s
 
 signal_base::~signal_base()
 {
-  if (impl_)
-  {
-    impl_->unreference();
-  }
 }
 
 void
@@ -298,13 +286,7 @@ signal_base::operator=(const signal_base& src)
   if (src.impl_ == impl_)
     return *this;
 
-  if (impl_)
-  {
-    impl_->unreference();
-  }
-
   impl_ = src.impl();
-  impl_->reference();
   return *this;
 }
 
@@ -314,25 +296,19 @@ signal_base::operator=(signal_base&& src)
   if (src.impl_ == impl_)
     return *this;
 
-  if (impl_)
-  {
-    impl_->unreference();
-  }
-
   src.notify_callbacks();
   impl_ = src.impl_;
   src.impl_ = nullptr;
 
   return *this;
 }
 
-internal::signal_impl*
+std::shared_ptr<internal::signal_impl>
 signal_base::impl() const
 {
   if (!impl_)
   {
-    impl_ = new internal::signal_impl;
-    impl_->reference(); // start with a reference count of 1
+    impl_ = std::make_shared<internal::signal_impl>();
   }
   return impl_;
 }
diff --git a/sigc++/signal_base.h b/sigc++/signal_base.h
@@ -21,6 +21,7 @@
 
 #include <cstddef>
 #include <list>
+#include <memory> //For std::shared_ptr<>
 #include <sigc++config.h>
 #include <sigc++/type_traits.h>
 #include <sigc++/trackable.h>
@@ -42,7 +43,9 @@ namespace internal
  * erase() to sweep() when the signal is being emitted. sweep() removes all
  * invalid slots from the list.
  */
-struct SIGC_API signal_impl : public notifiable
+struct SIGC_API signal_impl
+ : public notifiable,
+   public std::enable_shared_from_this<signal_impl>
 {
   using size_type = std::size_t;
   using slot_list = std::list<slot_base>;
@@ -64,34 +67,19 @@ struct SIGC_API signal_impl : public notifiable
   void operator delete(void* p);
 #endif
 
-  /// Increments the reference counter.
-  inline void reference() noexcept { ++ref_count_; }
-
   /// Increments the reference and execution counter.
   inline void reference_exec() noexcept
   {
-    ++ref_count_;
     ++exec_count_;
   }
 
-  /** Decrements the reference counter.
-   * The object is deleted when the reference counter reaches zero.
-   */
-  inline void unreference()
-  {
-    if (!(--ref_count_))
-      delete this;
-  }
-
   /** Decrements the reference and execution counter.
    * Invokes sweep() if the execution counter reaches zero and the
    * removal of one or more slots has been deferred.
    */
   inline void unreference_exec()
   {
-    if (!(--ref_count_))
-      delete this;
-    else if (!(--exec_count_) && deferred_)
+    if (!(--exec_count_) && deferred_)
       sweep();
   }
 
@@ -184,11 +172,6 @@ struct SIGC_API signal_impl : public notifiable
   std::list<slot_base> slots_;
 
 private:
-  /** Reference counter.
-   * The object is destroyed when @em ref_count_ reaches zero.
-   */
-  short ref_count_;
-
   /** Execution counter.
    * Indicates whether the signal is being emitted.
    */
@@ -204,7 +187,8 @@ struct SIGC_API signal_exec
   /** Increments the reference and execution counter of the parent sigc::signal_impl object.
    * @param sig The parent sigc::signal_impl object.
    */
-  inline signal_exec(const signal_impl* sig) noexcept : sig_(const_cast<signal_impl*>(sig))
+  inline signal_exec(const std::shared_ptr<signal_impl>& sig) noexcept
+  : sig_(sig)
   {
     sig_->reference_exec();
   }
@@ -220,7 +204,7 @@ struct SIGC_API signal_exec
 
 protected:
   /// The parent sigc::signal_impl object.
-  signal_impl* sig_;
+  const std::shared_ptr<signal_impl> sig_;
 };
 
 } /* namespace internal */
@@ -388,10 +372,10 @@ struct SIGC_API signal_base : public trackable
   /** Returns the signal_impl object encapsulating the list of slots.
    * @return The signal_impl object encapsulating the list of slots.
    */
-  internal::signal_impl* impl() const;
+  std::shared_ptr<internal::signal_impl> impl() const;
 
   /// The signal_impl object encapsulating the slot list.
-  mutable internal::signal_impl* impl_;
+  mutable std::shared_ptr<internal::signal_impl> impl_;
 };
 
 } // namespace sigc
