From 48fb6d2019057bb434ff7c31a66345007c43af90 Mon Sep 17 00:00:00 2001
From: lizheng <lizheng2@huawei.com>
Date: Wed, 22 Dec 2021 17:28:33 +0800
Subject: [PATCH] fix vsync dead lock

Change-Id: I306eb2c0c71213100df6faa6edeecdb7dfa1b571
Signed-off-by: lizheng <lizheng2@huawei.com>
---
 frameworks/vsync/src/vsync_callback_proxy.cpp |  5 ++-
 frameworks/vsync/src/vsync_helper_impl.cpp    |  4 ++-
 frameworks/vsync/src/vsync_manager.cpp        | 31 ++++++++++++++-----
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/frameworks/vsync/src/vsync_callback_proxy.cpp b/frameworks/vsync/src/vsync_callback_proxy.cpp
index 18ddbcfcdd..b8a29ae17e 100644
--- a/frameworks/vsync/src/vsync_callback_proxy.cpp
+++ b/frameworks/vsync/src/vsync_callback_proxy.cpp
@@ -31,7 +31,7 @@ VsyncCallbackProxy::VsyncCallbackProxy(const sptr<IRemoteObject>& impl)
 
 GSError VsyncCallbackProxy::OnVsync(int64_t timestamp)
 {
-    MessageOption opt;
+    MessageOption opt(MessageOption::TF_ASYNC);
     MessageParcel arg;
     MessageParcel ret;
 
@@ -53,8 +53,7 @@ GSError VsyncCallbackProxy::OnVsync(int64_t timestamp)
         return GSERROR_BINDER;
     }
 
-    int result = ret.ReadInt32();
-    GSError err = (GSError)ReturnValueTester::Get<int>(result);
+    GSError err = (GSError)ReturnValueTester::Get<int>(GSERROR_OK);
     if (err != GSERROR_OK) {
         VLOG_FAILURE_NO(err);
         return err;
diff --git a/frameworks/vsync/src/vsync_helper_impl.cpp b/frameworks/vsync/src/vsync_helper_impl.cpp
index e450de50c3..8b32b961f2 100644
--- a/frameworks/vsync/src/vsync_helper_impl.cpp
+++ b/frameworks/vsync/src/vsync_helper_impl.cpp
@@ -60,6 +60,7 @@ GSError VsyncClient::InitService()
 {
     std::lock_guard<std::mutex> lock(serviceMutex_);
     if (service_ == nullptr) {
+        ScopedBytrace func(__func__);
         auto sam = StaticCall::GetInstance()->GetSystemAbilityManager();
         if (sam == nullptr) {
             VLOG_FAILURE_RET(GSERROR_CONNOT_CONNECT_SAMGR);
@@ -146,6 +147,7 @@ GSError VsyncClient::Init(bool restart)
 GSError VsyncClient::InitListener()
 {
     while (listener_ == nullptr) {
+        ScopedBytrace func(__func__);
         auto vret = GSERROR_OK;
         {
             std::lock_guard<std::mutex> lock(serviceMutex_);
@@ -211,9 +213,9 @@ GSError VsyncClient::RequestFrameCallback(const struct FrameCallback &cb)
     {
         std::lock_guard<std::mutex> lockGuard(callbacksMapMutex_);
         callbacksMap_[vsyncID].push(ele);
-        InitListener();
     }
 
+    InitListener();
     VLOG_SUCCESS("RequestFrameCallback time: " VPUBI64 ", id: %{public}u", delayTime, vsyncID);
     return GSERROR_OK;
 }
diff --git a/frameworks/vsync/src/vsync_manager.cpp b/frameworks/vsync/src/vsync_manager.cpp
index 413d0c3674..73453f0127 100644
--- a/frameworks/vsync/src/vsync_manager.cpp
+++ b/frameworks/vsync/src/vsync_manager.cpp
@@ -15,6 +15,8 @@
 
 #include "vsync_manager.h"
 
+#include <scoped_bytrace.h>
+
 #include "vsync_callback_death_recipient.h"
 #include "vsync_callback_proxy.h"
 #include "vsync_log.h"
@@ -94,6 +96,7 @@ GSError VsyncManager::ListenVsync(sptr<IVsyncCallback>& cb)
         VLOGW("Failed to add death recipient");
     }
 
+    ScopedBytrace bytrace(__func__);
     std::lock_guard<std::mutex> lock(callbacksMutex_);
     callbacks_.push_back(cb);
     return GSERROR_OK;
@@ -101,6 +104,8 @@ GSError VsyncManager::ListenVsync(sptr<IVsyncCallback>& cb)
 
 GSError VsyncManager::RemoveVsync(sptr<IVsyncCallback>& callback)
 {
+    ScopedBytrace bytrace(__func__);
+    std::lock_guard<std::mutex> lock(callbacksMutex_);
     for (auto it = callbacks_.begin(); it != callbacks_.end(); it++) {
         if (*it == callback) {
             callbacks_.erase(it);
@@ -120,16 +125,28 @@ GSError VsyncManager::GetVsyncFrequency(uint32_t &freq)
 
 void VsyncManager::Callback(int64_t timestamp)
 {
-    std::lock_guard<std::mutex> lock(callbacksMutex_);
-
     using sptrIVsyncCallback = sptr<IVsyncCallback>;
-    std::list<sptrIVsyncCallback> okcbs;
-    for (const auto &cb : callbacks_) {
-        if (cb->OnVsync(timestamp) != GSERROR_BINDER) {
-            okcbs.push_back(cb);
+    std::list<sptrIVsyncCallback> okcbs, calling;
+    {
+        ScopedBytrace bytrace("callback 1");
+        std::lock_guard<std::mutex> lock(callbacksMutex_);
+        calling = callbacks_;
+    }
+
+    {
+        ScopedBytrace bytrace("callback 2");
+        for (const auto &cb : calling) {
+            if (cb->OnVsync(timestamp) != GSERROR_BINDER) {
+                okcbs.push_back(cb);
+            }
         }
     }
-    callbacks_ = okcbs;
+
+    {
+        ScopedBytrace bytrace("callback 3");
+        std::lock_guard<std::mutex> lock(callbacksMutex_);
+        callbacks_ = okcbs;
+    }
 }
 } // namespace Vsync
 } // namespace OHOS