From 137d3288abb75af3233e0a7271f5351132ddac3d Mon Sep 17 00:00:00 2001 From: "Alexander Kutsan (GitHub)" Date: Thu, 18 Jun 2020 18:12:58 +0300 Subject: [PATCH] Fix potential deadlock in PolicyHandler (#3383) SYNC-10345 There was a vulnerability in the PolicyHandler which causes a mutex deadlock. For example - MessageLoop thread of RpcService handles incoming messages. In case when SDL receives AllowSDLFunctionality notification, this thread calls OnAllowSDLFunctionalityNotification inside PolicyHandler. At some point of time this function captures accessor from AM which holds applications_list_lock_ there. At this moment thread AM Pool 0 of RequestController processes some RPC from queue and captures policy_manager_lock_ in PolicyHandler. After that at some moment thread AM Pool 0 tries to get application shared pointer from AM and locks itself as this mutex are already locked with thread MessageLoop. Also, MessageLoop thread at some moment tries to acquire policy_manager_lock_ and locks itself as this mutex are already locked with thread AM Pool 0, which is waiting for applications_list_lock_ to unlock. As a result we have a classical thread deadlock after which SDL stuck forewer. To avoid such situations, there was analyzed all bottlenecks related to applications_list_lock_ and its accessors. Accessors were scoped in several places to avoid similar deadlocks in future. Co-authored-by: Elvis Kuliiev --- .../src/policies/policy_handler.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 85d6858b0d0..646d523cf4b 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1310,18 +1310,22 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( #ifdef EXTERNAL_PROPRIETARY_MODE - DataAccessor accessor = - application_manager_.applications(); + ApplicationSet applications; + { + DataAccessor accessor = + application_manager_.applications(); + applications = accessor.GetData(); + } if (!is_allowed) { std::for_each( - accessor.GetData().begin(), - accessor.GetData().end(), + applications.begin(), + applications.end(), DeactivateApplication(device_handle, application_manager_.state_controller())); } else { std::for_each( - accessor.GetData().begin(), - accessor.GetData().end(), + applications.begin(), + applications.end(), SDLAlowedNotification(device_handle, policy_manager_.get(), application_manager_.state_controller()));