[Systeminfo] Prevention for unnecessary double mutex-locking 55/170555/2
authorSzymon Jastrzebski <s.jastrzebsk@partner.samsung.com>
Tue, 20 Feb 2018 14:09:48 +0000 (15:09 +0100)
committerSzymon Jastrzebski <s.jastrzebsk@partner.samsung.com>
Tue, 20 Feb 2018 14:43:42 +0000 (15:43 +0100)
The mutex locking may hang the application if all getters fail.

[Verification] Systeminfo TCT 100% pass rate + manually tested in console.

Change-Id: I270c6f5a5cb40f3975104476aa4e7de3179e0e64
Signed-off-by: Szymon Jastrzebski <s.jastrzebsk@partner.samsung.com>
src/systeminfo/systeminfo_sim_details_manager.cc

index 3fb1c228f229f75ba276cf671d9925b65c964d72..8b85d5b206bc24cd36ad13b13362f3037078e061 100644 (file)
@@ -146,6 +146,8 @@ PlatformResult SimDetailsManager::GatherSimInformation(TapiHandle* handle, picoj
     if (ret.IsError()) {
       return ret;
     }
+    // The variable used to determine if double mutex-locking is needed.
+    auto to_process_cached = to_process_;
     {
       // All props should be fetched synchronously, but sync function does not work
       std::lock_guard<std::mutex> lock_to_process(sim_to_process_mutex_);
@@ -178,8 +180,16 @@ PlatformResult SimDetailsManager::GatherSimInformation(TapiHandle* handle, picoj
         LoggerE("Failed getting iccid: %d", result);
       }
     }
-    // prevent returning not filled result
-    std::lock_guard<std::mutex> lock_sim(sim_info_mutex_);
+    // Here, the double mutex-locking may hang the application (thread) forever.
+    // This happens when none of above (4) getters succeeds (the to_process_ variable is not
+    // incremented).
+    // Usually, we would return and report an error, but the getPropertyValue and
+    // getPropertyValueArray methods do not invoke errorCallback with proper error type, thus silent
+    // error will be reported.
+    if (to_process_cached != to_process_) {
+      // Try to lock and wait for unlocking the sim_info_mutex_ by all registered callbacks.
+      std::lock_guard<std::mutex> lock_sim(sim_info_mutex_);
+    }
     // result will come from callbacks
     return PlatformResult(ErrorCode::NO_ERROR);
   }