Fix/usb handler thread safe stop (#3492)

* Wait for UsbHandler thread end in UsbHandler dtor

The Thread dedicated for handling libusb events should exit
successfully on it's own.

This guarantees that `libusb_close()` being called on all
devices, as well as `libusb_exit()` call is properly sequenced.

Thread exits in recommended by libusb doc way:
  1. set exit flag
  2. deregister hotplug callbacks, which will wake up
     `libusb_handle_events()` once again.

But this commit changes the way the Thread being joined. It
ensures that `join()` is called with `kNoStop` flag, which
prevents force stop using `pthread_cancel()`.

* Using atomic bool for shutdown_requested_ flag

Making UsbHandler::shutdown_requested_ flag atomic to guarantee
mutual access to it.

exchange() call is needed for helgrind to feel sure that there is
no data race.

* Remove redundant DCHECK

This assertion has been exposed by the current fix

3 files changed, 40 insertions, 5 deletions

diff --git a/src/components/application_manager/src/message_helper/message_helper.cc b/src/components/application_manager/src/message_helper/message_helper.cc
index b66742a7f5..ba1c726415 100644
--- a/src/components/application_manager/src/message_helper/message_helper.cc
+++ b/src/components/application_manager/src/message_helper/message_helper.cc
@@ -2684,9 +2684,9 @@ void MessageHelper::SendSystemRequestNotification(
 
   content[strings::params][strings::connection_key] = connection_key;
   PrintSmartObject(content);
-  DCHECK(app_mngr.GetRPCService().ManageMobileCommand(
+  app_mngr.GetRPCService().ManageMobileCommand(
       std::make_shared<smart_objects::SmartObject>(content),
-      commands::Command::SOURCE_SDL));
+      commands::Command::SOURCE_SDL);
 }
 
 void MessageHelper::SendLaunchApp(const uint32_t connection_key,
diff --git a/src/components/transport_manager/include/transport_manager/usb/libusb/usb_handler.h b/src/components/transport_manager/include/transport_manager/usb/libusb/usb_handler.h
index 9c16b49df6..eabb902379 100644
--- a/src/components/transport_manager/include/transport_manager/usb/libusb/usb_handler.h
+++ b/src/components/transport_manager/include/transport_manager/usb/libusb/usb_handler.h
@@ -37,6 +37,7 @@
 #define SRC_COMPONENTS_TRANSPORT_MANAGER_INCLUDE_TRANSPORT_MANAGER_USB_LIBUSB_USB_HANDLER_H_
 
 #include <libusb-1.0/libusb.h>
+#include <atomic>
 
 #include "transport_manager/transport_adapter/transport_adapter.h"
 #include "transport_manager/usb/libusb/platform_usb_device.h"
@@ -71,6 +72,11 @@ class UsbHandler {
   void SubmitControlTransfer(ControlTransferSequenceState* sequence_state);
   friend void UsbTransferSequenceCallback(libusb_transfer* transfer);
 
+  void RequestStopThread();
+  void DeregisterHotplugCallbacks();
+  void JoinAndDeleteThread();
+  void InvokeLibusbExit();
+
  private:
   class UsbHandlerDelegate : public threads::ThreadDelegate {
    public:
@@ -82,7 +88,7 @@ class UsbHandler {
     UsbHandler* handler_;
   };
 
-  bool shutdown_requested_;
+  std::atomic<bool> shutdown_requested_;
   threads::Thread* thread_;
 
   friend class UsbDeviceListener;
diff --git a/src/components/transport_manager/src/usb/libusb/usb_handler.cc b/src/components/transport_manager/src/usb/libusb/usb_handler.cc
index c8973dfcc1..09b1a017bb 100644
--- a/src/components/transport_manager/src/usb/libusb/usb_handler.cc
+++ b/src/components/transport_manager/src/usb/libusb/usb_handler.cc
@@ -89,8 +89,27 @@ UsbHandler::UsbHandler()
 }
 
 UsbHandler::~UsbHandler() {
-  shutdown_requested_ = true;
-  SDL_LOG_INFO("UsbHandler thread finished");
+  SDL_LOG_AUTO_TRACE();
+
+  // Notify Thread to stop on next iteration...
+  RequestStopThread();
+
+  // ... and unregister hotplug callbacks in order to wake
+  // libusb_handle_events()
+  DeregisterHotplugCallbacks();
+
+  // Now it is safe to join the Thread and free up resources
+  JoinAndDeleteThread();
+  InvokeLibusbExit();
+}
+
+void UsbHandler::RequestStopThread() {
+  SDL_LOG_AUTO_TRACE();
+  shutdown_requested_.store(true);
+}
+
+void UsbHandler::DeregisterHotplugCallbacks() {
+  SDL_LOG_AUTO_TRACE();
 
   if (libusb_context_) {
     // The libusb_hotplug_deregister_callback() wakes up blocking call of
@@ -99,10 +118,20 @@ UsbHandler::~UsbHandler() {
                                        arrived_callback_handle_);
     libusb_hotplug_deregister_callback(libusb_context_, left_callback_handle_);
   }
+}
+
+void UsbHandler::JoinAndDeleteThread() {
+  SDL_LOG_AUTO_TRACE();
 
+  // Let thread finish on its own
   thread_->Stop(threads::Thread::kThreadSoftStop);
   delete thread_->GetDelegate();
   threads::DeleteThread(thread_);
+  SDL_LOG_INFO("UsbHandler thread finished");
+}
+
+void UsbHandler::InvokeLibusbExit() {
+  SDL_LOG_AUTO_TRACE();
 
   if (libusb_context_) {
     libusb_exit(libusb_context_);