Fixup![Gamepad]fix coredump when gamepad disconnect. 13/308413/2
authorYu Yang <yangy.yu@samsung.com>
Thu, 21 Mar 2024 05:50:58 +0000 (13:50 +0800)
committerBot Blink <blinkbot@samsung.com>
Tue, 26 Mar 2024 02:00:12 +0000 (02:00 +0000)
Gamepad connect/disconnect callback is invoked in mainthread,
and gamepad read is invoked in polling thread.
when gamepad disconnect,
OCIGamepadItem will be accessed in mainthread and polling thread,
mainthread destoring OCIGamepadItem while polling thread is still use it,
that could make crash.
solution:
Moving all handle of OCIGamepadItem to polling thread to avoid issue.

Change-Id: I3bfda2112ee78b9cf7a16ed2e92d091182a73bba
Signed-off-by: Yu Yang <yangy.yu@samsung.com>
tizen_src/ewk/efl_integration/browser/gamepad/gamepad_platform_data_fetcher_tizen_tv.cc
tizen_src/ewk/efl_integration/browser/gamepad/gamepad_platform_data_fetcher_tizen_tv.h
tizen_src/ewk/efl_integration/browser/gamepad/oci_gamepad_item.cc
tizen_src/ewk/efl_integration/browser/gamepad/oci_gamepad_item.h

index fc0eaba..608527d 100644 (file)
@@ -90,20 +90,10 @@ void GamepadPlatformDataFetcherTizenTV::MapperTizenStyleGamepad(
 
 GamepadPlatformDataFetcherTizenTV::GamepadPlatformDataFetcherTizenTV() {
   gamepad_manager_ = nullptr;
-  if (Gamepad_Create()) {
-#if TIZEN_VERSION_AT_LEAST(8, 0, 0)
-    // initialize autoinput
-    VirtualKey_Initialize("gamepad-service");
-#endif
-    Initialize();
-  } else {
-    LOG(ERROR) << "[Gamepad_LOG] "
-                  "GamepadPlatformDataFetcherTizenTV::"
-                  "GamepadPlatformDataFetcherTizenTV() Gamepad_Create() ERROR "
-                  "!!!";
-  }
+  polling_runner_ = nullptr;
 }
 
+// invoked in polling thread
 GamepadPlatformDataFetcherTizenTV::~GamepadPlatformDataFetcherTizenTV() {
   CHECK(gamepad_manager_ != nullptr);
   gamepad_manager_->UnregisterCallback(this);
@@ -217,6 +207,26 @@ void GamepadPlatformDataFetcherTizenTV::GetGamepadData(
   }
 }
 
+// OnAddedToProvider is called in polling thread in initialize time
+// gamepad initialize should be done here instead of constructor.
+void GamepadPlatformDataFetcherTizenTV::OnAddedToProvider() {
+  polling_runner_ = base::SingleThreadTaskRunner::GetCurrentDefault();
+  DCHECK(polling_runner_);
+
+  if (Gamepad_Create()) {
+#if TIZEN_VERSION_AT_LEAST(8, 0, 0)
+    // initialize autoinput
+    VirtualKey_Initialize("gamepad-service");
+#endif
+    Initialize();
+  } else {
+    LOG(ERROR) << "[Gamepad_LOG] "
+                  "GamepadPlatformDataFetcherTizenTV::"
+                  "GamepadPlatformDataFetcherTizenTV() Gamepad_Create() ERROR "
+                  "!!!";
+  }
+}
+
 GamepadSource GamepadPlatformDataFetcherTizenTV::source() {
   return Factory::static_source();
 }
@@ -270,10 +280,54 @@ void GamepadPlatformDataFetcherTizenTV::InitMapping(size_t index) {
   UTF8toUTF16(pad.id, sizeof(pad.id), id, Gamepad::kIdLengthCap);
 }
 
+void GamepadPlatformDataFetcherTizenTV::HandleGamepadConnectionStatus(
+    int type,
+    const OCIDevInfo& deviceinfo) {
+  LOG(INFO) << " deviceinfo.UID " << deviceinfo.UID << " deviceinfo.name "
+            << deviceinfo.name;
+
+  switch (type) {
+    case OCI_EVENT_DEV_CONNECT: {
+      for (size_t i = 0; i < Gamepads::kItemsLengthCap; i++) {
+        if (gamepad_items_[i] == nullptr) {
+          LOG(INFO) << " connected";
+          gamepad_items_[i] = OCIGamepadItem::Create(
+              gamepad_manager_, &deviceinfo, &data_.items[i], i);
+          if (gamepad_items_[i]) {
+            InitMapping(i);
+            break;
+          }
+        }
+      }
+      break;
+    }
+
+    case OCI_EVENT_DEV_DISCONNECT: {
+      for (size_t i = 0; i < Gamepads::kItemsLengthCap; i++) {
+        if (gamepad_items_[i] != nullptr &&
+            (strcmp(gamepad_items_[i]->GetUID(), deviceinfo.UID) == 0)) {
+          LOG(INFO) << " gamepad_items_[" << i << "]->GetUID() "
+                    << gamepad_items_[i]->GetUID();
+          gamepad_items_[i]->Shutdown();
+          gamepad_items_[i] = nullptr;
+          break;
+        }
+      }
+      break;
+    }
+    default: {
+      break;
+    }
+  }
+}
 void GamepadPlatformDataFetcherTizenTV::t_OnCallback(int type,
                                                      void* pparam1,
                                                      void* pparam2,
                                                      void* pparam3) {
+  LOG(INFO) << "[Gamepad_LOG] t_OnCallback";
+  LOG(INFO) << type << " pparam1 " << reinterpret_cast<char*>(pparam1);
+  LOG(INFO) << type << " pparam2 " << reinterpret_cast<char*>(pparam2);
+
   int evType = 0;
   OCIDevInfo dev_info;
   std::string sType = "";
@@ -283,7 +337,6 @@ void GamepadPlatformDataFetcherTizenTV::t_OnCallback(int type,
   else if (type == OCI_EVENT_DEV_DISCONNECT)
     sType = " DISCONNECT ";
 
-  LOG(INFO) << sType << " pparam1 " << reinterpret_cast<char*>(pparam1);
 #if TIZEN_VERSION_AT_LEAST(8, 0, 0)
   strncpy(dev_info.UID, (char*)pparam1, OCI_SIZE_ID - 1);
   strncpy(dev_info.name, (char*)pparam2, OCI_SIZE_NAME - 1);
@@ -312,45 +365,13 @@ void GamepadPlatformDataFetcherTizenTV::t_OnCallback(int type,
   }
 #endif
 
-  switch (type) {
-    case OCI_EVENT_DEV_CONNECT: {
-      for (size_t i = 0; i < Gamepads::kItemsLengthCap; i++) {
-        if (gamepad_items_[i] == nullptr) {
-          LOG(INFO) << " connected";
-          gamepad_items_[i] = OCIGamepadItem::Create(
-              gamepad_manager_, &dev_info, &data_.items[i], i);
-          if (gamepad_items_[i]) {
-            InitMapping(i);
-            break;
-          }
-        } else {
-          continue;
-        }
-      }
-      break;
-    }
-
-    case OCI_EVENT_DEV_DISCONNECT: {
-      for (size_t i = 0; i < Gamepads::kItemsLengthCap; i++) {
-        if (gamepad_items_[i] != nullptr &&
-            (strcmp(gamepad_items_[i]->GetUID(), dev_info.UID) == 0)) {
-          LOG(INFO) << " gamepad_items_[" << i << "]->GetUID() "
-                    << gamepad_items_[i]->GetUID();
-          LOG(INFO) << " disconnected";
-          gamepad_items_[i]->Shutdown();
-          gamepad_items_[i] = nullptr;
-          break;
-        }
-      }
-      break;
-    }
-    case OCI_EVENT_DEV_STATUS: {
-      break;
-    }
-    default: {
-      break;
-    }
-  }
+  // operate gamepad in polling thread.
+  DCHECK(polling_runner_);
+  polling_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &GamepadPlatformDataFetcherTizenTV::HandleGamepadConnectionStatus,
+          base::Unretained(this), type, dev_info));
 }
 
 void GamepadPlatformDataFetcherTizenTV::PlayEffect(
index 61239c5..cab6a56 100644 (file)
@@ -61,6 +61,7 @@ class GamepadPlatformDataFetcherTizenTV
                     void* pparam3) override;
 
  private:
+  void OnAddedToProvider() override;
   void ReadTizenDeviceData(size_t index);
   void EnumerateTizenDevices();
 
@@ -75,6 +76,8 @@ class GamepadPlatformDataFetcherTizenTV
                    std::string str,
                    size_t str_length);
 
+  void HandleGamepadConnectionStatus(int type, const OCIDevInfo& deviceinfo);
+
   // Functions to map from device data to standard layout.
   static void MapperTizenStyleGamepad(const Gamepad& input, Gamepad* mapped);
 
@@ -84,6 +87,10 @@ class GamepadPlatformDataFetcherTizenTV
   gamepad::IGamepadManager* gamepad_manager_;
   std::array<std::unique_ptr<OCIGamepadItem>, Gamepads::kItemsLengthCap>
       gamepad_items_;
+
+  // Task runner to use for gamepad polling.
+  // it will be set in OnAddedToProvider.
+  scoped_refptr<base::SingleThreadTaskRunner> polling_runner_;
 };
 
 }  // namespace device
index 5420a15..572908e 100644 (file)
@@ -44,10 +44,11 @@ void closeLatencyFile() {
 size_t OCIGamepadItem::s_oci_gamepaditem_num_ = 0;
 
 // static function
-std::unique_ptr<OCIGamepadItem> OCIGamepadItem::Create(IGamepadManager* manager,
-                                                       OCIDevInfo* dev_info,
-                                                       Gamepad* pad,
-                                                       int index) {
+std::unique_ptr<OCIGamepadItem> OCIGamepadItem::Create(
+    IGamepadManager* manager,
+    const OCIDevInfo* dev_info,
+    Gamepad* pad,
+    int index) {
   if (manager == nullptr) {
     return nullptr;
   }
@@ -135,8 +136,9 @@ bool OCIGamepadItem::InitAxisRangeTizen() {
 }
 
 OCIGamepadItem::OCIGamepadItem(IGamepadManager* manager,
-                               OCIDevInfo* info,
+                               const OCIDevInfo* info,
                                Gamepad* pad) {
+  LOG(ERROR) << "[Gamepad_LOG]OCIGamepadItem construct";
   CHECK(info != nullptr);
   memcpy(&info_, info, sizeof(OCIDevInfo));
   CHECK(manager != nullptr);
@@ -166,6 +168,7 @@ OCIGamepadItem::OCIGamepadItem(IGamepadManager* manager,
 }
 
 OCIGamepadItem::~OCIGamepadItem() {
+  LOG(ERROR) << "[Gamepad_LOG][~OCIGamepadItem]";
   if (gamepad_ == nullptr)  // CreateDevice fail, need not destroy device.
     return;
   this->DestroyDevice();
index 088adf7..d5ea0a8 100644 (file)
@@ -51,7 +51,7 @@ class OCIGamepadItem : public gamepad::CGamepadCallback,
   // static function for create OCIGamepadItem, used by
   // GamepadPlatformDataFetcherTizenTV
   static std::unique_ptr<OCIGamepadItem> Create(IGamepadManager* manager,
-                                                OCIDevInfo* dev_info,
+                                                const OCIDevInfo* dev_info,
                                                 Gamepad* pad,
                                                 int index);
 
@@ -89,7 +89,7 @@ class OCIGamepadItem : public gamepad::CGamepadCallback,
   OCIControllerEvent events_[kOCIGamepadItemsLength];
 
   explicit OCIGamepadItem(IGamepadManager* manager,
-                          OCIDevInfo* info,
+                          const OCIDevInfo* info,
                           Gamepad* pad);
   bool CreateDevice();
   void DestroyDevice();