From 52f7eecc58dec08074ade7cea335bf91f88efb92 Mon Sep 17 00:00:00 2001 From: "Olha Vorobiova (GitHub)" <86727408+OlhaVorobiova@users.noreply.github.com> Date: Tue, 25 Jan 2022 21:15:11 +0200 Subject: Resolve a deadlock (#3828) * Fixed deadlock * Delete useless members in EventDispatcher --- .../event_engine/event_dispatcher.h | 9 ---- .../event_engine/event_dispatcher_impl.h | 27 ---------- .../src/event_engine/event_dispatcher_impl.cc | 63 ++++++---------------- .../src/request_controller_impl.cc | 2 - .../src/request_timeout_handler_impl.cc | 3 ++ 5 files changed, 19 insertions(+), 85 deletions(-) (limited to 'src') diff --git a/src/components/application_manager/include/application_manager/event_engine/event_dispatcher.h b/src/components/application_manager/include/application_manager/event_engine/event_dispatcher.h index b232875f43..0656372f47 100644 --- a/src/components/application_manager/include/application_manager/event_engine/event_dispatcher.h +++ b/src/components/application_manager/include/application_manager/event_engine/event_dispatcher.h @@ -61,15 +61,6 @@ class EventDispatcher { int32_t hmi_correlation_id, EventObserver& observer) = 0; - /* - * @brief Unsubscribes the observer from specific event - * - * @param event_id The event ID to subscribe for - * @param hmi_correlation_id The event HMI correlation ID - */ - virtual void remove_observer(const Event::EventID& event_id, - const int32_t hmi_correlation_id) = 0; - /* * @brief Unsubscribes the observer from specific event * diff --git a/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h b/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h index ffe659a07c..b4cfead5fa 100644 --- a/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h +++ b/src/components/application_manager/include/application_manager/event_engine/event_dispatcher_impl.h @@ -68,12 +68,6 @@ class EventDispatcherImpl : public EventDispatcher { EventObserverMap get_observers() const { return observers_event_; } - MobileEventObserverMap get_mobile_observers() const { - return mobile_observers_event_; - } - ObserverVector get_observers_list() const { - return observers_; - } #endif // BUILD_TESTS /* @@ -83,9 +77,6 @@ class EventDispatcherImpl : public EventDispatcher { */ void raise_event(const Event& event) OVERRIDE; - void remove_observer(const Event::EventID& event_id, - const int32_t hmi_correlation_id) OVERRIDE; - /* * @brief Subscribe the observer to event * @@ -149,32 +140,14 @@ class EventDispatcherImpl : public EventDispatcher { void remove_mobile_observer(EventObserver& observer) OVERRIDE; private: - /* - * @brief removes observer - * when occurs unsubscribe from event - * @param observer to be removed - */ - void remove_observer_from_vector(EventObserver& observer); - - /* - * @brief removes observer - * when occurs unsubscribe from event - * @param observer to be removed - */ - void remove_mobile_observer_from_vector(EventObserver& observer); - DISALLOW_COPY_AND_ASSIGN(EventDispatcherImpl); private: // Members section - sync_primitives::Lock state_lock_; - sync_primitives::Lock mobile_state_lock_; sync_primitives::RecursiveLock observer_lock_; sync_primitives::RecursiveLock mobile_observer_lock_; EventObserverMap observers_event_; MobileEventObserverMap mobile_observers_event_; - ObserverVector observers_; - ObserverVector mobile_observers_; }; } // namespace event_engine diff --git a/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc b/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc index 0d7dc9cfe0..441e3831f8 100644 --- a/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc +++ b/src/components/application_manager/src/event_engine/event_dispatcher_impl.cc @@ -45,9 +45,9 @@ EventDispatcherImpl::~EventDispatcherImpl() {} void EventDispatcherImpl::raise_event(const Event& event) { ObserverVector observers; - AutoLock observer_lock(observer_lock_); + { - AutoLock state_lock(state_lock_); + AutoLock observer_lock(observer_lock_); // check if event is notification if (hmi_apis::messageType::notification == event.smart_object_type()) { @@ -65,7 +65,6 @@ void EventDispatcherImpl::raise_event(const Event& event) { while (!observers.empty()) { EventObserver* temp = *observers.begin(); observers.erase(observers.begin()); - AutoUnlock unlock_observer(observer_lock); if (temp->IncrementReferenceCount()) { temp->HandleOnEvent(event); @@ -77,7 +76,7 @@ void EventDispatcherImpl::raise_event(const Event& event) { void EventDispatcherImpl::add_observer(const Event::EventID& event_id, int32_t hmi_correlation_id, EventObserver& observer) { - AutoLock auto_lock(state_lock_); + AutoLock auto_lock(observer_lock_); observers_event_[event_id][hmi_correlation_id].push_back(&observer); } @@ -92,19 +91,9 @@ struct IdCheckFunctor { const unsigned long target_id; }; -void EventDispatcherImpl::remove_observer(const Event::EventID& event_id, - const int32_t hmi_correlation_id) { - AutoLock auto_lock(state_lock_); - auto& observers = observers_event_[event_id][hmi_correlation_id]; - for (auto observer : observers) { - remove_observer_from_vector(*observer); - } -} - void EventDispatcherImpl::remove_observer(const Event::EventID& event_id, EventObserver& observer) { - remove_observer_from_vector(observer); - AutoLock auto_lock(state_lock_); + AutoLock auto_lock(observer_lock_); ObserversMap::iterator it = observers_event_[event_id].begin(); for (; observers_event_[event_id].end() != it; ++it) { @@ -118,7 +107,8 @@ void EventDispatcherImpl::remove_observer(const Event::EventID& event_id, } void EventDispatcherImpl::remove_observer(EventObserver& observer) { - remove_observer_from_vector(observer); + AutoLock auto_lock(observer_lock_); + EventObserverMap::iterator event_map = observers_event_.begin(); for (; observers_event_.end() != event_map; ++event_map) { @@ -126,41 +116,32 @@ void EventDispatcherImpl::remove_observer(EventObserver& observer) { } } -void EventDispatcherImpl::remove_observer_from_vector(EventObserver& observer) { - AutoLock auto_lock(observer_lock_); - - observers_.erase( - std::remove_if( - observers_.begin(), observers_.end(), IdCheckFunctor(observer.id())), - observers_.end()); -} - // Mobile Events void EventDispatcherImpl::raise_mobile_event(const MobileEvent& event) { - AutoLock observer_lock(mobile_observer_lock_); + ObserverVector mobile_observers; + { - AutoLock state_lock(mobile_state_lock_); + AutoLock observer_lock(mobile_observer_lock_); // check if event is notification if (mobile_apis::messageType::notification == event.smart_object_type()) { const uint32_t notification_correlation_id = 0; - mobile_observers_ = + mobile_observers = mobile_observers_event_[event.id()][notification_correlation_id]; } if (mobile_apis::messageType::response == event.smart_object_type()) { - mobile_observers_ = + mobile_observers = mobile_observers_event_[event.id()] [event.smart_object_correlation_id()]; } } // Call observers - while (!mobile_observers_.empty()) { - EventObserver* temp = *mobile_observers_.begin(); - mobile_observers_.erase(mobile_observers_.begin()); - AutoUnlock unlock_observer(observer_lock); + while (!mobile_observers.empty()) { + EventObserver* temp = *mobile_observers.begin(); + mobile_observers.erase(mobile_observers.begin()); if (temp->IncrementReferenceCount()) { temp->HandleOnEvent(event); @@ -173,14 +154,13 @@ void EventDispatcherImpl::add_mobile_observer( const MobileEvent::MobileEventID& event_id, int32_t mobile_correlation_id, EventObserver& observer) { - AutoLock auto_lock(mobile_state_lock_); + AutoLock auto_lock(mobile_observer_lock_); mobile_observers_event_[event_id][mobile_correlation_id].push_back(&observer); } void EventDispatcherImpl::remove_mobile_observer( const MobileEvent::MobileEventID& event_id, EventObserver& observer) { - remove_mobile_observer_from_vector(observer); - AutoLock auto_lock(mobile_state_lock_); + AutoLock auto_lock(mobile_observer_lock_); ObserversMap::iterator it = mobile_observers_event_[event_id].begin(); for (; mobile_observers_event_[event_id].end() != it; ++it) { @@ -194,7 +174,6 @@ void EventDispatcherImpl::remove_mobile_observer( } void EventDispatcherImpl::remove_mobile_observer(EventObserver& observer) { - remove_mobile_observer_from_vector(observer); MobileEventObserverMap::iterator event_map = mobile_observers_event_.begin(); for (; mobile_observers_event_.end() != event_map; ++event_map) { @@ -202,15 +181,5 @@ void EventDispatcherImpl::remove_mobile_observer(EventObserver& observer) { } } -void EventDispatcherImpl::remove_mobile_observer_from_vector( - EventObserver& observer) { - AutoLock auto_lock(mobile_observer_lock_); - - mobile_observers_.erase( - std::remove_if( - observers_.begin(), observers_.end(), IdCheckFunctor(observer.id())), - observers_.end()); -} - } // namespace event_engine } // namespace application_manager diff --git a/src/components/application_manager/src/request_controller_impl.cc b/src/components/application_manager/src/request_controller_impl.cc index 76d4a70330..b73ebc0b74 100644 --- a/src/components/application_manager/src/request_controller_impl.cc +++ b/src/components/application_manager/src/request_controller_impl.cc @@ -335,8 +335,6 @@ void RequestControllerImpl::TerminateRequest(const uint32_t correlation_id, return; } if (force_terminate || request->request()->AllowedToTerminate()) { - event_dispatcher_.remove_observer( - static_cast(function_id), correlation_id); waiting_for_response_.RemoveRequest(request); if (RequestInfo::HMIRequest == request->request_type()) { request_timeout_handler_.RemoveRequest(request->requestId()); diff --git a/src/components/application_manager/src/request_timeout_handler_impl.cc b/src/components/application_manager/src/request_timeout_handler_impl.cc index 1ac72393a0..b8e0e67746 100644 --- a/src/components/application_manager/src/request_timeout_handler_impl.cc +++ b/src/components/application_manager/src/request_timeout_handler_impl.cc @@ -49,6 +49,7 @@ RequestTimeoutHandlerImpl::RequestTimeoutHandlerImpl( void RequestTimeoutHandlerImpl::AddRequest(const uint32_t hmi_correlation_id, const Request& request) { + sync_primitives::AutoLock lock(requests_lock_); requests_.insert(std::make_pair(hmi_correlation_id, request)); } @@ -101,6 +102,8 @@ void RequestTimeoutHandlerImpl::HandleOnEvent( } const auto hmi_corr_id = message[strings::msg_params][strings::request_id].asUInt(); + + sync_primitives::AutoLock lock(requests_lock_); auto it = requests_.find(hmi_corr_id); if (it != requests_.end()) { const auto& request = it->second; -- cgit v1.2.1