From cef01a32a1bafd450adcbc733920f31e52b209dc Mon Sep 17 00:00:00 2001
From: Hang Lu <quic_hangl@quicinc.com>
Date: Wed, 30 Aug 2023 18:27:25 +0800
Subject: [PATCH] Remove the active deletion action of callbacks

The active deletion action will cause binder blocked in some corner
cases. As serviceDied will take care of its deletion, remove the active
deletion has no side effect and could fix the issue.

Test: none
Bug: 296817256
Change-Id: Ib30b3910202e6bbd86ac59e0e69fbeb0a890dd38
---
 health/aidl/default/Health.cpp | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/health/aidl/default/Health.cpp b/health/aidl/default/Health.cpp
index 1d8cc132a7..352a658fb8 100644
--- a/health/aidl/default/Health.cpp
+++ b/health/aidl/default/Health.cpp
@@ -235,15 +235,6 @@ std::optional<bool> Health::ShouldKeepScreenOn() {
     return healthd_config_->screen_on(&props);
 }
 
-namespace {
-bool IsDeadObjectLogged(const ndk::ScopedAStatus& ret) {
-    if (ret.isOk()) return false;
-    if (ret.getStatus() == ::STATUS_DEAD_OBJECT) return true;
-    LOG(ERROR) << "Cannot call healthInfoChanged on callback: " << ret.getDescription();
-    return false;
-}
-}  // namespace
-
 //
 // Subclass helpers / overrides
 //
@@ -287,8 +278,10 @@ ndk::ScopedAStatus Health::registerCallback(const std::shared_ptr<IHealthInfoCal
         return ndk::ScopedAStatus::ok();
     }
 
-    if (auto res = callback->healthInfoChanged(health_info); IsDeadObjectLogged(res)) {
-        (void)unregisterCallback(callback);
+    auto res = callback->healthInfoChanged(health_info);
+    if (!res.isOk()) {
+        LOG(DEBUG) << "Cannot call healthInfoChanged:" << res.getDescription()
+                   << ". Do nothing here if callback is dead as it will be cleaned up later.";
     }
     return ndk::ScopedAStatus::ok();
 }
@@ -335,13 +328,13 @@ ndk::ScopedAStatus Health::update() {
 void Health::OnHealthInfoChanged(const HealthInfo& health_info) {
     // Notify all callbacks
     std::unique_lock<decltype(callbacks_lock_)> lock(callbacks_lock_);
-    // is_dead notifies a callback and return true if it is dead.
-    auto is_dead = [&](const auto& linked) {
+    for (const auto& linked : callbacks_) {
         auto res = linked->callback()->healthInfoChanged(health_info);
-        return IsDeadObjectLogged(res);
-    };
-    auto it = std::remove_if(callbacks_.begin(), callbacks_.end(), is_dead);
-    callbacks_.erase(it, callbacks_.end());  // calls unlinkToDeath on deleted callbacks.
+        if (!res.isOk()) {
+            LOG(DEBUG) << "Cannot call healthInfoChanged:" << res.getDescription()
+                       << ". Do nothing here if callback is dead as it will be cleaned up later.";
+        }
+    }
     lock.unlock();
 
     // Let HalHealthLoop::OnHealthInfoChanged() adjusts uevent / wakealarm periods