search

smartdevicelink / sdl_core

SmartDeviceLink In-Vehicle Software and Sample HMI
BSD 3-Clause "New" or "Revised" License
241 stars 244 forks source link

SDL Core crash in RegisterAppInterface if app disconnects during resumption #3720

Closed jacobkeeler closed 3 years ago

jacobkeeler commented 3 years ago

Bug Report

An invalid read is seen in RegisterAppInterfaceRequest sometimes after performing resumption. This is due to the "send_response" callback in the Run function, which can be triggered after the RAI request is already destroyed. In the worst case, this callback can even cause a crash.

Valgrind description:

==11742== Invalid read of size 8
==11742==    at 0x154A0313: sdl_rpc_plugin::commands::RegisterAppInterfaceRequest::SendRegisterAppInterfaceResponseToMobile(sdl_rpc_plugin::commands::RegisterAppInterfaceRequest::ApplicationType, std::shared_ptr<utils::Callable>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (register_app_interface_request.cc:991)
==11742==    by 0x1549A743: sdl_rpc_plugin::commands::RegisterAppInterfaceRequest::Run()::{lambda(mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)#2}::operator()(mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) const (register_app_interface_request.cc:722)
==11742==    by 0x154ABB6C: std::_Function_handler<void (mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), sdl_rpc_plugin::commands::RegisterAppInterfaceRequest::Run()::{lambda(mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)#2}>::_M_invoke(std::_Any_data const&, mobile_apis::Result::eType&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (std_function.h:300)
==11742==    by 0xF0E171: std::function<void (mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()(mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (std_function.h:688)
==11742==    by 0xEFEA6D: resumption::ResumptionDataProcessorImpl::FinalizeResumption(std::function<void (mobile_apis::Result::eType, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)> const&, unsigned int) (resumption_data_processor_impl.cc:312)
==11742==    by 0xEFE2AF: resumption::ResumptionDataProcessorImpl::ProcessResponseFromHMI(ns_smart_device_link::ns_smart_objects::SmartObject const&, hmi_apis::FunctionID::eType, int) (resumption_data_processor_impl.cc:305)
==11742==    by 0xEFF775: resumption::ResumptionDataProcessorImpl::on_event(application_manager::event_engine::Event const&) (resumption_data_processor_impl.cc:344)
==11742==    by 0x2080D17: application_manager::event_engine::EventDispatcherImpl::raise_event(application_manager::event_engine::Event const&) (event_dispatcher_impl.cc:69)
==11742==    by 0x1565D585: application_manager::event_engine::Event::raise(application_manager::event_engine::EventDispatcher&) (event.cc:44)
==11742==    by 0x15598B17: sdl_rpc_plugin::commands::TTSSetGlobalPropertiesResponse::Run() (tts_set_global_properties_response.cc:63)
==11742==    by 0xF54D96: application_manager::rpc_service::RPCServiceImpl::ManageHMICommand(std::shared_ptr<ns_smart_device_link::ns_smart_objects::SmartObject>, application_manager::commands::Command::CommandSource, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_service_impl.cc:392)
==11742==    by 0xF20400: application_manager::rpc_handler::RPCHandlerImpl::ProcessMessageFromHMI(std::shared_ptr<application_manager::Message>) (rpc_handler_impl.cc:202)
==11742==  Address 0x1ab6cd10 is 32 bytes inside a block of size 664 free'd
==11742==    at 0x483CFBF: operator delete(void*) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==11742==    by 0x15495997: sdl_rpc_plugin::commands::RegisterAppInterfaceRequest::~RegisterAppInterfaceRequest() (register_app_interface_request.cc:148)
==11742==    by 0x153F501D: std::_Sp_counted_ptr<sdl_rpc_plugin::commands::RegisterAppInterfaceRequest*, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:377)
==11742==    by 0xD83AB3: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:155)
==11742==    by 0xD83258: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:730)
==11742==    by 0xE027BF: std::__shared_ptr<application_manager::commands::Command, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1169)
==11742==    by 0xE027DF: std::shared_ptr<application_manager::commands::Command>::~shared_ptr() (shared_ptr.h:103)
==11742==    by 0xE9BB05: application_manager::request_controller::RequestInfo::~RequestInfo() (request_info.h:68)
==11742==    by 0xE9EB8D: application_manager::request_controller::MobileRequestInfo::~MobileRequestInfo() (request_info.h:144)
==11742==    by 0xE979AC: void __gnu_cxx::new_allocator<application_manager::request_controller::MobileRequestInfo>::destroy<application_manager::request_controller::MobileRequestInfo>(application_manager::request_controller::MobileRequestInfo*) (new_allocator.h:153)
==11742==    by 0xE97928: void std::allocator_traits<std::allocator<application_manager::request_controller::MobileRequestInfo> >::destroy<application_manager::request_controller::MobileRequestInfo>(std::allocator<application_manager::request_controller::MobileRequestInfo>&, application_manager::request_controller::MobileRequestInfo*) (alloc_traits.h:497)
==11742==    by 0xE97158: std::_Sp_counted_ptr_inplace<application_manager::request_controller::MobileRequestInfo, std::allocator<application_manager::request_controller::MobileRequestInfo>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:557)
==11742==  Block was alloc'd at
==11742==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==11742==    by 0x153E6845: application_manager::DefaultCommandCreator<sdl_rpc_plugin::commands::RegisterAppInterfaceRequest>::create(std::shared_ptr<ns_smart_device_link::ns_smart_objects::SmartObject> const&) const (command_factory.h:136)
==11742==    by 0x153D62A1: sdl_rpc_plugin::MobileCommandFactory::CreateCommand(std::shared_ptr<ns_smart_device_link::ns_smart_objects::SmartObject> const&, application_manager::commands::Command::CommandSource) (mobile_command_factory.cc:578)
==11742==    by 0x1538CB6D: sdl_rpc_plugin::SDLCommandFactory::CreateCommand(std::shared_ptr<ns_smart_device_link::ns_smart_objects::SmartObject> const&, application_manager::commands::Command::CommandSource) (sdl_command_factory.cc:62)
==11742==    by 0xF5206A: application_manager::rpc_service::RPCServiceImpl::ManageMobileCommand(std::shared_ptr<ns_smart_device_link::ns_smart_objects::SmartObject>, application_manager::commands::Command::CommandSource, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) (rpc_service_impl.cc:205)
==11742==    by 0xF1F031: application_manager::rpc_handler::RPCHandlerImpl::ProcessMessageFromMobile(std::shared_ptr<application_manager::Message>) (rpc_handler_impl.cc:148)
==11742==    by 0xF21103: application_manager::rpc_handler::RPCHandlerImpl::Handle(application_manager::rpc_handler::impl::MessageFromMobile) (rpc_handler_impl.cc:224)
==11742==    by 0xF36125: threads::MessageLoopThread<utils::PrioritizedQueue<application_manager::rpc_handler::impl::MessageFromMobile> >::LoopThreadDelegate::DrainQue() (message_loop_thread.h:201)
==11742==    by 0xF35CD4: threads::MessageLoopThread<utils::PrioritizedQueue<application_manager::rpc_handler::impl::MessageFromMobile> >::LoopThreadDelegate::threadMain() (message_loop_thread.h:184)
==11742==    by 0x1FD7038: threads::Thread::threadFunc(void*)::{lambda(threads::Thread*)#1}::operator()(threads::Thread*) const (thread_posix.cc:76)
==11742==    by 0x1FD7791: threads::Thread::threadFunc(void*) (thread_posix.cc:106)
==11742==    by 0x4869608: start_thread (pthread_create.c:477)

Core logs from crash:

TRACE [09 Jun 2021 13:26:16,140][140487057667840][Commands] /home/jacobkeeler/sdl_core/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc:476 commands::RegisterAppInterfaceRequest::Run: Exit
...
DEBUG [09 Jun 2021 13:26:26,134][140487049275136][Commands] /home/jacobkeeler/sdl_core/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc:716 commands::RegisterAppInterfaceRequest::Run: Invoking lambda callback for: 0x7fc580010ec0
TRACE [09 Jun 2021 13:26:26,134][140487049275136][Commands] /home/jacobkeeler/sdl_core/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/register_app_interface_request.cc:866 commands::RegisterAppInterfaceRequest::SendRegisterAppInterfaceResponseToMobile: Enter
DEBUG [09 Jun 2021 13:26:26,135][140487049275136][SDLMain] /home/jacobkeeler/sdl_core/src/appMain/life_cycle_impl.cc:240 {anonymous}::sig_handler: SIGSEGV signal has been caught
Reproduction Steps
  1. Start SDL Core with valgrind: valgrind ./smartDeviceLinkCore
  2. Connect app
  3. Send 5 AddCommands from app
  4. Disconnect app
  5. Reconnect app with proper hashID
Expected Behavior

No memory errors are generated

Observed Behavior

An Invalid read of size 8 error is logged by valgrind

OS & Version Information
jacobkeeler commented 3 years ago

Full crash log: SmartDeviceLinkCore.log

jacobkeeler commented 3 years ago

Seems the crash is actually separate from the invalid read. The crash occurs when an app disconnects during the resumption process.

Reproduction Steps
  1. Connect app to SDL HMI
  2. Send SetGlobalProperties with menuIcon from app
  3. Go to Settings->RPC Control->"Your app" and set UI.SetGlobalProperties to DO_NOT_RESPOND, save
  4. Disconnect app
  5. Reconnect app with proper hashID
  6. Disconnect app during resumption process
Expected Behavior

No issues occur, app does not appear in app list

Observed Behavior

After 10 seconds, SDL Core crashes

jacobkeeler commented 3 years ago

Closing with the merge of #3721