libaurum: Delete double lock for atspi API thread safe call 49/261049/4
authorWoochanlee <wc0917.lee@samsung.com>
Fri, 9 Jul 2021 05:13:29 +0000 (14:13 +0900)
committerkim hosang <hosang12.kim@samsung.com>
Fri, 9 Jul 2021 08:02:10 +0000 (08:02 +0000)
atspi API is safe through a unique lock at there

Change-Id: I817b49086fb61f63fb98bd490c86e719f1b7b03f

libaurum/inc/Impl/Accessibility/AtspiAccessibleWatcher.h
libaurum/inc/Impl/Accessibility/AtspiWrapper.h
libaurum/src/Impl/Accessibility/AtspiAccessibleNode.cc
libaurum/src/Impl/Accessibility/AtspiAccessibleWatcher.cc
libaurum/src/Impl/Accessibility/AtspiWrapper.cc

index 5c1f9b247b4933a39271ef85eb6360d248aed82b..d0590b47c5907ddcd057d4506439883596347e04 100644 (file)
@@ -231,5 +231,7 @@ private:
     /**
      * @brief TBD
      */
-       static std::vector<std::shared_ptr<A11yEventInfo>>    mEventQueue;
+    static std::vector<std::shared_ptr<A11yEventInfo>>    mEventQueue;
+
+    static std::mutex mMutex;
 };
index 869ba6a0ec53c818c309f6bb23f1304fc06fb760..0bd3b0269a785683e03521be29155220be8bbe9e 100644 (file)
@@ -29,8 +29,6 @@ public:
     static AtspiAccessible *Atspi_accessible_get_application (AtspiAccessible *node, GError **error);
     static void Atspi_accessible_clear_cache (AtspiAccessible *node);
 
-    static void lock();
-    static void unlock();
 private:
     static std::recursive_mutex mMutex;
     //static std::unique_lock<std::mutex> mLock;
index a5641f9e213dff9875af14e71c010f01e712acf4..e196ab42816bc422763e310f68f47833e8fb3452 100644 (file)
@@ -28,26 +28,20 @@ AtspiAccessibleNode::~AtspiAccessibleNode()
 
 int AtspiAccessibleNode::getChildCount() const
 {
-    AtspiWrapper::lock();
     if (!isValid()) {
-        AtspiWrapper::unlock();
         return 0;
     }
     int count = AtspiWrapper::Atspi_accessible_get_child_count(mNode, NULL);
-    AtspiWrapper::unlock();
     if (count <= 0) return 0;
     return count;
 }
 
 std::shared_ptr<AccessibleNode> AtspiAccessibleNode::getChildAt(int index) const
 {
-    AtspiWrapper::lock();
     if (!isValid()) {
-        AtspiWrapper::unlock();
         return std::make_shared<AtspiAccessibleNode>(nullptr);
     }
     AtspiAccessible *rawChild = AtspiWrapper::Atspi_accessible_get_child_at_index(mNode, index, NULL);
-    AtspiWrapper::unlock();
     return std::make_shared<AtspiAccessibleNode>(rawChild);
 }
 
@@ -64,13 +58,10 @@ std::vector<std::shared_ptr<AccessibleNode>> AtspiAccessibleNode::getChildren()
 
 std::shared_ptr<AccessibleNode> AtspiAccessibleNode::getParent() const
 {
-    AtspiWrapper::lock();
     if (!isValid()) {
-        AtspiWrapper::unlock();
         return std::make_shared<AtspiAccessibleNode>(nullptr);
     }
     AtspiAccessible *rawParent = AtspiWrapper::Atspi_accessible_get_parent(mNode, NULL);
-    AtspiWrapper::unlock();
     return std::make_shared<AtspiAccessibleNode>(rawParent);
 }
 
@@ -96,8 +87,6 @@ void* AtspiAccessibleNode::getRawHandler(void) const
 
 void AtspiAccessibleNode::refresh()
 {
-    AtspiWrapper::lock();
-
     AtspiWrapper::Atspi_accessible_clear_cache(mNode);
 
     if (isValid()) {
@@ -175,17 +164,13 @@ void AtspiAccessibleNode::refresh()
     } else {
         setFeatureProperty(ATSPI_STATE_INVALID);
     }
-    AtspiWrapper::unlock();
 }
 
 std::vector<std::string> AtspiAccessibleNode::getActions() const
 {
-    AtspiWrapper::lock();
-
     std::vector<std::string> result{};
     AtspiAction *action;
     if (!isValid()) {
-        AtspiWrapper::unlock();
         return result;
     }
 
@@ -203,18 +188,14 @@ std::vector<std::string> AtspiAccessibleNode::getActions() const
         g_object_unref(action);
     }
 
-    AtspiWrapper::unlock();
-
     return result;
 }
 
 bool AtspiAccessibleNode::doAction(std::string actionName)
 {
-    AtspiWrapper::lock();
     AtspiAction *action;
 
     if (!isValid()) {
-        AtspiWrapper::unlock();
         return false;
     }
 
@@ -226,7 +207,6 @@ bool AtspiAccessibleNode::doAction(std::string actionName)
         for (a = 0; a < n_actions; a++) {
             char *action_name = AtspiWrapper::Atspi_action_get_action_name(action, a, NULL);
             if (!action_name) {
-                AtspiWrapper::unlock();
                  return false;
             }
 
@@ -234,22 +214,18 @@ bool AtspiAccessibleNode::doAction(std::string actionName)
                 AtspiWrapper::Atspi_action_do_action(action, a, NULL);
                 g_free(action_name);
                 g_object_unref(action);
-                AtspiWrapper::unlock();
                 return true;
             }
             g_free(action_name);
         }
         g_object_unref(action);
     }
-    AtspiWrapper::unlock();
     return false;
 }
 
 void AtspiAccessibleNode::setValue(std::string text)
 {
-    AtspiWrapper::lock();
     if (!isValid()){
-        AtspiWrapper::unlock();
         return;
     }
 
@@ -261,7 +237,6 @@ void AtspiAccessibleNode::setValue(std::string text)
         AtspiWrapper::Atspi_editable_text_insert_text(iface, 0, text.c_str(), text.length(),
                                         NULL);
     }
-    AtspiWrapper::unlock();
 }
 
 void AtspiAccessibleNode::setFeatureProperty(AtspiStateType type)
index a58a89f00477321e36319701862e7643722cb1fe..79de9e0b19e432e32a4e689a0b92ffe22f61e2a4 100644 (file)
@@ -13,6 +13,7 @@
 
 std::vector<std::shared_ptr<A11yEventInfo>> AtspiAccessibleWatcher::mEventQueue;
 GThread *AtspiAccessibleWatcher::mEventThread = nullptr;
+std::mutex AtspiAccessibleWatcher::mMutex = std::mutex{};
 
 static bool iShowingNode(AtspiAccessible *node)
 {
@@ -139,10 +140,8 @@ AtspiAccessibleWatcher::~AtspiAccessibleWatcher()
 
 void AtspiAccessibleWatcher::onAtspiEvents(AtspiEvent *event, void *user_data)
 {
-    AtspiWrapper::lock();
     if (!event->source)
     {
-        AtspiWrapper::unlock();
         return;
     }
     char *name = NULL, *pkg = NULL;
@@ -158,7 +157,9 @@ void AtspiAccessibleWatcher::onAtspiEvents(AtspiEvent *event, void *user_data)
     else
         pkg = strdup("");
 
+    mMutex.lock();
     mEventQueue.push_back(std::make_shared<A11yEventInfo>(std::string(event->type), std::string(name), std::string(pkg)));
+    mMutex.unlock();
 
     if (!strcmp(event->type, "object:state-changed:defunct")) {
          instance->onObjectDefunct(
@@ -166,8 +167,6 @@ void AtspiAccessibleWatcher::onAtspiEvents(AtspiEvent *event, void *user_data)
     }
     if (name) free(name);
     if (pkg) free(pkg);
-
-    AtspiWrapper::unlock();
 }
 
 void AtspiAccessibleWatcher::print_debug()
@@ -228,37 +227,29 @@ void AtspiAccessibleWatcher::onObjectDefunct(AtspiAccessible *node)
 
 int AtspiAccessibleWatcher::getApplicationCount(void) const
 {
-    AtspiWrapper::lock();
-
     AtspiAccessible *root = AtspiWrapper::Atspi_get_desktop(0);
     int nchild = AtspiWrapper::Atspi_accessible_get_child_count(root, NULL);
     g_object_unref(root);
 
-    AtspiWrapper::unlock();
-
     if (nchild <= 0) return 0;
     return nchild;
 }
 
 std::shared_ptr<AccessibleApplication> AtspiAccessibleWatcher::getApplicationAt(int index) const
 {
-    AtspiWrapper::lock();
     AtspiAccessible *root = AtspiWrapper::Atspi_get_desktop(0);
     AtspiAccessible *child = AtspiWrapper::Atspi_accessible_get_child_at_index(root, index, NULL);
     g_object_unref(root);
-    AtspiWrapper::unlock();
     return std::make_shared<AtspiAccessibleApplication>(std::make_shared<AtspiAccessibleNode>(child));
 }
 
 std::vector<std::shared_ptr<AccessibleApplication>> AtspiAccessibleWatcher::getApplications(void) const
 {
-    AtspiWrapper::lock();
     std::vector<std::shared_ptr<AccessibleApplication>> ret{};
     AtspiAccessible *root = AtspiWrapper::Atspi_get_desktop(0);
     int nchild = AtspiWrapper::Atspi_accessible_get_child_count(root, NULL);
     if (nchild <= 0) {
         g_object_unref(root);
-        AtspiWrapper::unlock();
         return ret;
     }
 
@@ -269,7 +260,6 @@ std::vector<std::shared_ptr<AccessibleApplication>> AtspiAccessibleWatcher::getA
         }
     }
     g_object_unref(root);
-    AtspiWrapper::unlock();
     return ret;
 }
 
@@ -278,9 +268,9 @@ std::vector<std::shared_ptr<AccessibleApplication>> AtspiAccessibleWatcher::getA
 
 bool AtspiAccessibleWatcher::executeAndWaitForEvents(const Runnable *cmd, const A11yEvent type, const int timeout)
 {
-    AtspiWrapper::lock();
+    mMutex.lock();
     mEventQueue.clear();
-    AtspiWrapper::unlock();
+    mMutex.unlock();
     if (cmd)
         cmd->run();
 
@@ -289,10 +279,10 @@ bool AtspiAccessibleWatcher::executeAndWaitForEvents(const Runnable *cmd, const
        while (true)
     {
         std::vector<std::shared_ptr<A11yEventInfo>> localEvents;
-        AtspiWrapper::lock();
+        mMutex.lock();
         localEvents.assign(mEventQueue.begin(), mEventQueue.end());
         mEventQueue.clear();
-        AtspiWrapper::unlock();
+        mMutex.unlock();
 
         if (!localEvents.empty())
         {
index cf76dcd8c7311e5489e993d1cabf9d6ad22af2f4..109d0ccb79cf58b051fdd00328e2da2e1a9e99bf 100644 (file)
@@ -1,7 +1,6 @@
 #include "AtspiWrapper.h"
 
 std::recursive_mutex AtspiWrapper::mMutex = std::recursive_mutex{};
-//std::unique_lock<std::mutex> AtspiWrapper::mLock = std::unique_lock<std::mutex>(mMutex, std::defer_lock);
 
 GArray *AtspiWrapper::Atspi_state_set_get_states(AtspiStateSet *set)
 {
@@ -140,12 +139,3 @@ void AtspiWrapper::Atspi_accessible_clear_cache (AtspiAccessible *node)
     std::unique_lock<std::recursive_mutex> lock(mMutex);
     return atspi_accessible_clear_cache(node);
 }
-void AtspiWrapper::lock()
-{
-    mMutex.lock();
-}
-
-void AtspiWrapper::unlock()
-{
-    mMutex.unlock();
-}