diff --git a/webrtc/base/asyncinvoker.cc b/webrtc/base/asyncinvoker.cc index cabd8b506d..a143e25f19 100644 --- a/webrtc/base/asyncinvoker.cc +++ b/webrtc/base/asyncinvoker.cc @@ -20,6 +20,7 @@ AsyncInvoker::AsyncInvoker() : invocation_complete_(false, false) {} AsyncInvoker::~AsyncInvoker() { destroying_ = true; + SignalInvokerDestroyed(); // Messages for this need to be cleared *before* our destructor is complete. MessageQueueManager::Clear(this); // And we need to wait for any invocations that are still in progress on @@ -125,9 +126,8 @@ NotifyingAsyncClosureBase::NotifyingAsyncClosureBase( calling_thread_(calling_thread) { calling_thread->SignalQueueDestroyed.connect( this, &NotifyingAsyncClosureBase::CancelCallback); - // Note: We don't need to listen for the invoker being destroyed, because it - // will wait for this closure to be destroyed (and pending_invocations_ to go - // to 0) before its destructor completes. + invoker->SignalInvokerDestroyed.connect( + this, &NotifyingAsyncClosureBase::CancelCallback); } NotifyingAsyncClosureBase::~NotifyingAsyncClosureBase() { diff --git a/webrtc/base/asyncinvoker.h b/webrtc/base/asyncinvoker.h index 27f6a9fec0..ed61243974 100644 --- a/webrtc/base/asyncinvoker.h +++ b/webrtc/base/asyncinvoker.h @@ -144,6 +144,9 @@ class AsyncInvoker : public MessageHandler { // behavior is desired, call Flush() before destroying this object. void Flush(Thread* thread, uint32_t id = MQID_ANY); + // Signaled when this object is destructed. + sigslot::signal0<> SignalInvokerDestroyed; + private: void OnMessage(Message* msg) override; void DoInvoke(const Location& posted_from, diff --git a/webrtc/base/sigslot.h b/webrtc/base/sigslot.h index 652d63c0d2..45e0da216b 100644 --- a/webrtc/base/sigslot.h +++ b/webrtc/base/sigslot.h @@ -394,8 +394,7 @@ namespace sigslot { protected: typedef std::list< _opaque_connection > connections_list; - _signal_base() : _signal_base_interface(&_signal_base::do_slot_disconnect, &_signal_base::do_slot_duplicate), - m_current_iterator(m_connected_slots.end()) + _signal_base() : _signal_base_interface(&_signal_base::do_slot_disconnect, &_signal_base::do_slot_duplicate) { } @@ -408,8 +407,7 @@ namespace sigslot { _signal_base& operator= (_signal_base const& that); public: - _signal_base(const _signal_base& o) : _signal_base_interface(&_signal_base::do_slot_disconnect, &_signal_base::do_slot_duplicate), - m_current_iterator(m_connected_slots.end()) { + _signal_base(const _signal_base& o) : _signal_base_interface(&_signal_base::do_slot_disconnect, &_signal_base::do_slot_duplicate) { lock_block lock(this); for (const auto& connection : o.m_connected_slots) { @@ -462,14 +460,7 @@ namespace sigslot { { if(it->getdest() == pclass) { - // If we're currently using this iterator, - // don't erase it and invalidate it yet; set a - // flag to do so afterwards. - if (m_current_iterator == it) { - m_erase_current_iterator = true; - } else { - m_connected_slots.erase(it); - } + m_connected_slots.erase(it); pclass->signal_disconnect(static_cast< _signal_base_interface* >(this)); return; } @@ -493,15 +484,8 @@ namespace sigslot { if(it->getdest() == pslot) { - // If we're currently using this iterator, - // don't erase it and invalidate it yet; set a - // flag to do so afterwards. - if (self->m_current_iterator == it) { - self->m_erase_current_iterator = true; - } else { - self->m_connected_slots.erase(it); - } - } + self->m_connected_slots.erase(it); + } it = itNext; } @@ -527,12 +511,7 @@ namespace sigslot { protected: connections_list m_connected_slots; - - // Used to handle a slot being disconnected while a signal is - // firing (iterating m_connected_slots). - connections_list::iterator m_current_iterator; - bool m_erase_current_iterator = false; - }; + }; template class has_slots : public has_slots_interface, public mt_policy @@ -626,22 +605,16 @@ namespace sigslot { void emit(Args... args) { lock_block lock(this); - this->m_current_iterator = - this->m_connected_slots.begin(); - while (this->m_current_iterator != - this->m_connected_slots.end()) { - _opaque_connection const& conn = - *this->m_current_iterator; - conn.emit(args...); - if (this->m_erase_current_iterator) { - this->m_current_iterator = - this->m_connected_slots.erase( - this->m_current_iterator); - this->m_erase_current_iterator = false; - } else { - ++(this->m_current_iterator); - } - } + typename connections_list::const_iterator it = this->m_connected_slots.begin(); + typename connections_list::const_iterator itEnd = this->m_connected_slots.end(); + + while(it != itEnd) + { + _opaque_connection const& conn = *it; + ++it; + + conn.emit(args...); + } } void operator()(Args... args) diff --git a/webrtc/base/sigslot_unittest.cc b/webrtc/base/sigslot_unittest.cc index 33f0e6a8d9..e860fe3373 100644 --- a/webrtc/base/sigslot_unittest.cc +++ b/webrtc/base/sigslot_unittest.cc @@ -272,50 +272,3 @@ TEST(SigslotTest, CopyConnectedSlot) { signal(); EXPECT_EQ(1, copied_receiver.signal_count()); } - -// Just used for the test below. -class Disconnector : public sigslot::has_slots<> { - public: - Disconnector(SigslotReceiver<>* receiver1, SigslotReceiver<>* receiver2) - : receiver1_(receiver1), receiver2_(receiver2) {} - - void Connect(sigslot::signal<>* signal) { - signal_ = signal; - signal->connect(this, &Disconnector::Disconnect); - } - - private: - void Disconnect() { - receiver1_->Disconnect(); - receiver2_->Disconnect(); - signal_->disconnect(this); - } - - sigslot::signal<>* signal_; - SigslotReceiver<>* receiver1_; - SigslotReceiver<>* receiver2_; -}; - -// Test that things work as expected if a signal is disconnected from a slot -// while it's firing. -TEST(SigslotTest, DisconnectFromSignalWhileFiring) { - sigslot::signal<> signal; - SigslotReceiver<> receiver1; - SigslotReceiver<> receiver2; - SigslotReceiver<> receiver3; - Disconnector disconnector(&receiver1, &receiver2); - - // From this ordering, receiver1 should receive the signal, then the - // disconnector will be invoked, causing receiver2 to be disconnected before - // it receives the signal. And receiver2 should also receive the signal, - // since it was never disconnected. - receiver1.Connect(&signal); - disconnector.Connect(&signal); - receiver2.Connect(&signal); - receiver3.Connect(&signal); - signal(); - - EXPECT_EQ(1, receiver1.signal_count()); - EXPECT_EQ(0, receiver2.signal_count()); - EXPECT_EQ(1, receiver3.signal_count()); -}