commit 4717314d74cfae8485b1864d61647b4f49f58ff7
author Devin Moore <devinmoore@google.com> Mon Mar 18 22:37:53 2024 +0000
committer Android Build Coastguard Worker <android-build-coastguard-worker@google.com> Thu Jun 13 05:18:35 2024 +0000
tree ce4a2bfffdcd2dfaed02cc6136a50825745434e3
parent abc118d19406fdffb543dc416ac8a431a86b0a2a

Track DeathNotifier cookie lifetime

Keep a set of cookies to signal when the DeathNotifier cookie is valid.

Test: cd frameworks/av/media/libmediaplayerservice && atest -p
Test: atest CtsCameraTestCases:android.hardware.camera2.cts.RecordingTest
Test: adb shell kill -9 `pid media.swcodec` # during RecordingTest
Bug: 319210610
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:55128e7467c39633d6b69a62b9e47096fadc5d3d)
Merged-In: Id113b8af0708c3f93e6ddef90751692ac0f32855
Change-Id: Id113b8af0708c3f93e6ddef90751692ac0f32855

media/libmediaplayerservice/DeathNotifier.cpp

diff --git a/media/libmediaplayerservice/DeathNotifier.cpp b/media/libmediaplayerservice/DeathNotifier.cpp
index ab22f67..241c52d 100644
--- a/media/libmediaplayerservice/DeathNotifier.cpp
+++ b/media/libmediaplayerservice/DeathNotifier.cpp
@@ -17,11 +17,18 @@
 //#define LOG_NDEBUG 0
 #define LOG_TAG "MediaPlayerService-DeathNotifier"
 #include <android-base/logging.h>
+#include <map>
 
 #include "DeathNotifier.h"
 
 namespace android {
 
+// Only dereference the cookie if it's valid (if it's in this set)
+// Only used with ndk
+static uintptr_t sCookieKeyCounter = 0;
+static std::map<uintptr_t, wp<DeathNotifier::DeathRecipient>> sCookies;
+static std::mutex sCookiesMutex;
+
 class DeathNotifier::DeathRecipient :
         public IBinder::DeathRecipient,
         public hardware::hidl_death_recipient {
@@ -44,13 +51,32 @@
     }
 
     static void OnBinderDied(void *cookie) {
-        DeathRecipient *thiz = (DeathRecipient *)cookie;
-        thiz->mNotify();
+        std::unique_lock<std::mutex> guard(sCookiesMutex);
+        if (auto it = sCookies.find(reinterpret_cast<uintptr_t>(cookie)); it != sCookies.end()) {
+            sp<DeathRecipient> recipient = it->second.promote();
+            sCookies.erase(it);
+            guard.unlock();
+
+            if (recipient) {
+                LOG(INFO) << "Notifying DeathRecipient from OnBinderDied.";
+                recipient->mNotify();
+            } else {
+                LOG(INFO) <<
+                    "Tried to notify DeathRecipient from OnBinderDied but could not promote.";
+            }
+        }
     }
 
     AIBinder_DeathRecipient *getNdkRecipient() {
         return mNdkRecipient.get();;
     }
+    ~DeathRecipient() {
+        // lock must be taken so object is not used in OnBinderDied"
+        std::lock_guard<std::mutex> guard(sCookiesMutex);
+        sCookies.erase(mCookieKey);
+    }
+
+    uintptr_t mCookieKey;
 
 private:
     Notify mNotify;
@@ -73,8 +99,15 @@
       : mService{std::in_place_index<3>, service},
         mDeathRecipient{new DeathRecipient(notify)} {
     mDeathRecipient->initNdk();
+    {
+        std::lock_guard<std::mutex> guard(sCookiesMutex);
+        mDeathRecipient->mCookieKey = sCookieKeyCounter++;
+        sCookies[mDeathRecipient->mCookieKey] = mDeathRecipient;
+    }
     AIBinder_linkToDeath(
-            service.get(), mDeathRecipient->getNdkRecipient(), mDeathRecipient.get());
+            service.get(),
+            mDeathRecipient->getNdkRecipient(),
+            reinterpret_cast<void*>(mDeathRecipient->mCookieKey));
 }
 
 DeathNotifier::DeathNotifier(DeathNotifier&& other)
@@ -94,10 +127,11 @@
         std::get<2>(mService)->unlinkToDeath(mDeathRecipient);
         break;
     case 3:
+
         AIBinder_unlinkToDeath(
                 std::get<3>(mService).get(),
                 mDeathRecipient->getNdkRecipient(),
-                mDeathRecipient.get());
+                reinterpret_cast<void*>(mDeathRecipient->mCookieKey));
         break;
     default:
         CHECK(false) << "Corrupted service type during destruction.";

media/libmediaplayerservice/DeathNotifier.h

diff --git a/media/libmediaplayerservice/DeathNotifier.h b/media/libmediaplayerservice/DeathNotifier.h
index 24e45a3..0fd7c65 100644
--- a/media/libmediaplayerservice/DeathNotifier.h
+++ b/media/libmediaplayerservice/DeathNotifier.h
@@ -37,10 +37,11 @@
     DeathNotifier(DeathNotifier&& other);
     ~DeathNotifier();
 
+    class DeathRecipient;
+
 private:
     std::variant<std::monostate, sp<IBinder>, sp<HBase>, ::ndk::SpAIBinder> mService;
 
-    class DeathRecipient;
     sp<DeathRecipient> mDeathRecipient;
 };