diff --git a/webrtc/base/sigslot.h b/webrtc/base/sigslot.h index 45e0da216b..652d63c0d2 100644 --- a/webrtc/base/sigslot.h +++ b/webrtc/base/sigslot.h @@ -394,7 +394,8 @@ 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) + _signal_base() : _signal_base_interface(&_signal_base::do_slot_disconnect, &_signal_base::do_slot_duplicate), + m_current_iterator(m_connected_slots.end()) { } @@ -407,7 +408,8 @@ 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) { + _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()) { lock_block lock(this); for (const auto& connection : o.m_connected_slots) { @@ -460,7 +462,14 @@ namespace sigslot { { if(it->getdest() == pclass) { - m_connected_slots.erase(it); + // 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); + } pclass->signal_disconnect(static_cast< _signal_base_interface* >(this)); return; } @@ -484,8 +493,15 @@ namespace sigslot { if(it->getdest() == pslot) { - self->m_connected_slots.erase(it); - } + // 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); + } + } it = itNext; } @@ -511,7 +527,12 @@ 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 @@ -605,16 +626,22 @@ namespace sigslot { void emit(Args... args) { lock_block lock(this); - 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...); - } + 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); + } + } } void operator()(Args... args) diff --git a/webrtc/base/sigslot_unittest.cc b/webrtc/base/sigslot_unittest.cc index e860fe3373..94d0d4687f 100644 --- a/webrtc/base/sigslot_unittest.cc +++ b/webrtc/base/sigslot_unittest.cc @@ -272,3 +272,50 @@ 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 +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()); +}