Skip to content

Commit

Permalink
Fix potential deadlock in PolicyHandler (#3383)
Browse files Browse the repository at this point in the history
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 <ekuliiev@luxoft.com>
  • Loading branch information
LuxoftAKutsan and EKuliiev authored Jun 18, 2020
1 parent baf63ca commit 137d328
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions src/components/application_manager/src/policies/policy_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1310,18 +1310,22 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification(

#ifdef EXTERNAL_PROPRIETARY_MODE

DataAccessor<ApplicationSet> accessor =
application_manager_.applications();
ApplicationSet applications;
{
DataAccessor<ApplicationSet> 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()));
Expand Down

0 comments on commit 137d328

Please sign in to comment.