ndk: Add way to manage ownership of linkToDeath cookie

Bug: 181971563
Bug: 197721058
Test: m, modified existing test
Change-Id: Ib9b4ab41fd7929f9ba56e112c8a5b58966040771
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 23db59e..83bf9be 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -373,6 +373,12 @@
     return clazz->getInterfaceDescriptorUtf8();
 }
 
+AIBinder_DeathRecipient::TransferDeathRecipient::~TransferDeathRecipient() {
+    if (mOnUnlinked != nullptr) {
+        mOnUnlinked(mCookie);
+    }
+}
+
 void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinder>& who) {
     CHECK(who == mWho) << who.unsafe_get() << "(" << who.get_refs() << ") vs " << mWho.unsafe_get()
                        << " (" << mWho.get_refs() << ")";
@@ -394,7 +400,7 @@
 }
 
 AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied)
-    : mOnDied(onDied) {
+    : mOnDied(onDied), mOnUnlinked(nullptr) {
     CHECK(onDied != nullptr);
 }
 
@@ -412,10 +418,12 @@
     std::lock_guard<std::mutex> l(mDeathRecipientsMutex);
 
     sp<TransferDeathRecipient> recipient =
-            new TransferDeathRecipient(binder, cookie, this, mOnDied);
+            new TransferDeathRecipient(binder, cookie, this, mOnDied, mOnUnlinked);
 
     status_t status = binder->linkToDeath(recipient, cookie, 0 /*flags*/);
     if (status != STATUS_OK) {
+        // When we failed to link, the destructor of TransferDeathRecipient runs here, which
+        // ensures that mOnUnlinked is called before we return with an error from this method.
         return PruneStatusT(status);
     }
 
@@ -448,6 +456,10 @@
     return STATUS_NAME_NOT_FOUND;
 }
 
+void AIBinder_DeathRecipient::setOnUnlinked(AIBinder_DeathRecipient_onBinderUnlinked onUnlinked) {
+    mOnUnlinked = onUnlinked;
+}
+
 // start of C-API methods
 
 AIBinder* AIBinder_new(const AIBinder_Class* clazz, void* args) {
@@ -689,6 +701,15 @@
     return ret;
 }
 
+void AIBinder_DeathRecipient_setOnUnlinked(AIBinder_DeathRecipient* recipient,
+                                           AIBinder_DeathRecipient_onBinderUnlinked onUnlinked) {
+    if (recipient == nullptr) {
+        return;
+    }
+
+    recipient->setOnUnlinked(onUnlinked);
+}
+
 void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient* recipient) {
     if (recipient == nullptr) {
         return;
diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h
index 6509545..9fb5c1d 100644
--- a/libs/binder/ndk/ibinder_internal.h
+++ b/libs/binder/ndk/ibinder_internal.h
@@ -148,8 +148,14 @@
     struct TransferDeathRecipient : ::android::IBinder::DeathRecipient {
         TransferDeathRecipient(const ::android::wp<::android::IBinder>& who, void* cookie,
                                const ::android::wp<AIBinder_DeathRecipient>& parentRecipient,
-                               const AIBinder_DeathRecipient_onBinderDied onDied)
-            : mWho(who), mCookie(cookie), mParentRecipient(parentRecipient), mOnDied(onDied) {}
+                               const AIBinder_DeathRecipient_onBinderDied onDied,
+                               const AIBinder_DeathRecipient_onBinderUnlinked onUnlinked)
+            : mWho(who),
+              mCookie(cookie),
+              mParentRecipient(parentRecipient),
+              mOnDied(onDied),
+              mOnUnlinked(onUnlinked) {}
+        ~TransferDeathRecipient();
 
         void binderDied(const ::android::wp<::android::IBinder>& who) override;
 
@@ -165,11 +171,13 @@
         // This is kept separately from AIBinder_DeathRecipient in case the death recipient is
         // deleted while the death notification is fired
         const AIBinder_DeathRecipient_onBinderDied mOnDied;
+        const AIBinder_DeathRecipient_onBinderUnlinked mOnUnlinked;
     };
 
     explicit AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied);
     binder_status_t linkToDeath(const ::android::sp<::android::IBinder>&, void* cookie);
     binder_status_t unlinkToDeath(const ::android::sp<::android::IBinder>& binder, void* cookie);
+    void setOnUnlinked(AIBinder_DeathRecipient_onBinderUnlinked onUnlinked);
 
    private:
     // When the user of this API deletes a Bp object but not the death recipient, the
@@ -180,4 +188,5 @@
     std::mutex mDeathRecipientsMutex;
     std::vector<::android::sp<TransferDeathRecipient>> mDeathRecipients;
     AIBinder_DeathRecipient_onBinderDied mOnDied;
+    AIBinder_DeathRecipient_onBinderUnlinked mOnUnlinked;
 };
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index b881c2c..43533c5 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -319,9 +319,9 @@
 /**
  * Registers for notifications that the associated binder is dead. The same death recipient may be
  * associated with multiple different binders. If the binder is local, then no death recipient will
- * be given (since if the local process dies, then no recipient will exist to recieve a
+ * be given (since if the local process dies, then no recipient will exist to receive a
  * transaction). The cookie is passed to recipient in the case that this binder dies and can be
- * null. The exact cookie must also be used to unlink this transaction (see AIBinder_linkToDeath).
+ * null. The exact cookie must also be used to unlink this transaction (see AIBinder_unlinkToDeath).
  * This function may return a binder transaction failure. The cookie can be used both for
  * identification and holding user data.
  *
@@ -348,6 +348,10 @@
  * If the binder dies, it will automatically unlink. If the binder is deleted, it will be
  * automatically unlinked.
  *
+ * Be aware that it is not safe to immediately deallocate the cookie when this call returns. If you
+ * need to clean up the cookie, you should do so in the onUnlinked callback, which can be set using
+ * AIBinder_DeathRecipient_setOnUnlinked.
+ *
  * Available since API level 29.
  *
  * \param binder the binder object to remove a previously linked death recipient from.
@@ -568,6 +572,22 @@
 typedef void (*AIBinder_DeathRecipient_onBinderDied)(void* cookie) __INTRODUCED_IN(29);
 
 /**
+ * This function is intended for cleaning up the data in the provided cookie, and it is executed
+ * when the DeathRecipient is unlinked. When the DeathRecipient is unlinked due to a death receipt,
+ * this method is called after the call to onBinderDied.
+ *
+ * This method is called once for each binder that is unlinked. Hence, if the same cookie is passed
+ * to multiple binders, then the caller is responsible for reference counting the cookie.
+ *
+ * See also AIBinder_linkToDeath/AIBinder_unlinkToDeath.
+ *
+ * Available since API level 33.
+ *
+ * \param cookie the cookie passed to AIBinder_linkToDeath.
+ */
+typedef void (*AIBinder_DeathRecipient_onBinderUnlinked)(void* cookie) __INTRODUCED_IN(33);
+
+/**
  * Creates a new binder death recipient. This can be attached to multiple different binder objects.
  *
  * Available since API level 29.
@@ -580,9 +600,47 @@
         AIBinder_DeathRecipient_onBinderDied onBinderDied) __INTRODUCED_IN(29);
 
 /**
+ * Set the callback to be called when this DeathRecipient is unlinked from a binder. The callback is
+ * called in the following situations:
+ *
+ *  1. If the binder died, shortly after the call to onBinderDied.
+ *  2. If the binder is explicitly unlinked with AIBinder_unlinkToDeath or
+ *     AIBinder_DeathRecipient_delete.
+ *  3. During or shortly after the AIBinder_linkToDeath call if it returns an error.
+ *
+ * It is guaranteed that the callback is called exactly once for each call to linkToDeath unless the
+ * process is aborted before the binder is unlinked.
+ *
+ * Be aware that when the binder is explicitly unlinked, it is not guaranteed that onUnlinked has
+ * been called before the call to AIBinder_unlinkToDeath or AIBinder_DeathRecipient_delete returns.
+ * For example, if the binder dies concurrently with a call to AIBinder_unlinkToDeath, the binder is
+ * not unlinked until after the death notification is delivered, even if AIBinder_unlinkToDeath
+ * returns before that happens.
+ *
+ * This method should be called before linking the DeathRecipient to a binder because the function
+ * pointer is cached. If you change it after linking to a binder, it is unspecified whether the old
+ * binder will call the old or new onUnlinked callback.
+ *
+ * The onUnlinked argument may be null. In this case, no notification is given when the binder is
+ * unlinked.
+ *
+ * Available since API level 33.
+ *
+ * \param recipient the DeathRecipient to set the onUnlinked callback for.
+ * \param onUnlinked the callback to call when a binder is unlinked from recipient.
+ */
+void AIBinder_DeathRecipient_setOnUnlinked(AIBinder_DeathRecipient* recipient,
+                                           AIBinder_DeathRecipient_onBinderUnlinked onUnlinked)
+        __INTRODUCED_IN(33);
+
+/**
  * Deletes a binder death recipient. It is not necessary to call AIBinder_unlinkToDeath before
  * calling this as these will all be automatically unlinked.
  *
+ * Be aware that it is not safe to immediately deallocate the cookie when this call returns. If you
+ * need to clean up the cookie, you should do so in the onUnlinked callback, which can be set using
+ * AIBinder_DeathRecipient_setOnUnlinked.
+ *
  * Available since API level 29.
  *
  * \param recipient the binder to delete (previously created with AIBinder_DeathRecipient_new).
diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt
index ac892db..8605686 100644
--- a/libs/binder/ndk/libbinder_ndk.map.txt
+++ b/libs/binder/ndk/libbinder_ndk.map.txt
@@ -144,6 +144,7 @@
 LIBBINDER_NDK33 { # introduced=33
   global:
     AIBinder_Class_disableInterfaceTokenHeader;
+    AIBinder_DeathRecipient_setOnUnlinked;
     AParcel_marshal;
     AParcel_unmarshal;
 };
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index b5c06e9..d1ff4de 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -375,9 +375,16 @@
             << "Service failed to shut down.";
 }
 
+struct DeathRecipientCookie {
+    std::function<void(void)>*onDeath, *onUnlink;
+};
 void LambdaOnDeath(void* cookie) {
-    auto onDeath = static_cast<std::function<void(void)>*>(cookie);
-    (*onDeath)();
+    auto funcs = static_cast<DeathRecipientCookie*>(cookie);
+    (*funcs->onDeath)();
+};
+void LambdaOnUnlink(void* cookie) {
+    auto funcs = static_cast<DeathRecipientCookie*>(cookie);
+    (*funcs->onUnlink)();
 };
 TEST(NdkBinder, DeathRecipient) {
     using namespace std::chrono_literals;
@@ -389,26 +396,46 @@
 
     std::mutex deathMutex;
     std::condition_variable deathCv;
-    bool deathRecieved = false;
+    bool deathReceived = false;
 
     std::function<void(void)> onDeath = [&] {
         std::cerr << "Binder died (as requested)." << std::endl;
-        deathRecieved = true;
+        deathReceived = true;
         deathCv.notify_one();
     };
 
-    AIBinder_DeathRecipient* recipient = AIBinder_DeathRecipient_new(LambdaOnDeath);
+    std::mutex unlinkMutex;
+    std::condition_variable unlinkCv;
+    bool unlinkReceived = false;
+    bool wasDeathReceivedFirst = false;
 
-    EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(&onDeath)));
+    std::function<void(void)> onUnlink = [&] {
+        std::cerr << "Binder unlinked (as requested)." << std::endl;
+        wasDeathReceivedFirst = deathReceived;
+        unlinkReceived = true;
+        unlinkCv.notify_one();
+    };
+
+    DeathRecipientCookie cookie = {&onDeath, &onUnlink};
+
+    AIBinder_DeathRecipient* recipient = AIBinder_DeathRecipient_new(LambdaOnDeath);
+    AIBinder_DeathRecipient_setOnUnlinked(recipient, LambdaOnUnlink);
+
+    EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(&cookie)));
 
     // the binder driver should return this if the service dies during the transaction
     EXPECT_EQ(STATUS_DEAD_OBJECT, foo->die());
 
     foo = nullptr;
 
-    std::unique_lock<std::mutex> lock(deathMutex);
-    EXPECT_TRUE(deathCv.wait_for(lock, 1s, [&] { return deathRecieved; }));
-    EXPECT_TRUE(deathRecieved);
+    std::unique_lock<std::mutex> lockDeath(deathMutex);
+    EXPECT_TRUE(deathCv.wait_for(lockDeath, 1s, [&] { return deathReceived; }));
+    EXPECT_TRUE(deathReceived);
+
+    std::unique_lock<std::mutex> lockUnlink(unlinkMutex);
+    EXPECT_TRUE(deathCv.wait_for(lockUnlink, 1s, [&] { return unlinkReceived; }));
+    EXPECT_TRUE(unlinkReceived);
+    EXPECT_TRUE(wasDeathReceivedFirst);
 
     AIBinder_DeathRecipient_delete(recipient);
     AIBinder_decStrong(binder);
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index b03ed49..e1d01ef 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -31,8 +31,10 @@
 use std::convert::TryInto;
 use std::ffi::{c_void, CString};
 use std::fmt;
+use std::mem;
 use std::os::unix::io::AsRawFd;
 use std::ptr;
+use std::sync::Arc;
 
 /// A strong reference to a Binder remote object.
 ///
@@ -378,13 +380,17 @@
             // Safety: `SpIBinder` guarantees that `self` always contains a
             // valid pointer to an `AIBinder`. `recipient` can always be
             // converted into a valid pointer to an
-            // `AIBinder_DeathRecipient`. Any value is safe to pass as the
-            // cookie, although we depend on this value being set by
-            // `get_cookie` when the death recipient callback is called.
+            // `AIBinder_DeathRecipient`.
+            //
+            // The cookie is also the correct pointer, and by calling new_cookie,
+            // we have created a new ref-count to the cookie, which linkToDeath
+            // takes ownership of. Once the DeathRecipient is unlinked for any
+            // reason (including if this call fails), the onUnlinked callback
+            // will consume that ref-count.
             sys::AIBinder_linkToDeath(
                 self.as_native_mut(),
                 recipient.as_native_mut(),
-                recipient.get_cookie(),
+                recipient.new_cookie(),
             )
         })
     }
@@ -552,10 +558,20 @@
 }
 
 /// Rust wrapper around DeathRecipient objects.
+///
+/// The cookie in this struct represents an Arc<F> for the owned callback.
+/// This struct owns a ref-count of it, and so does every binder that we
+/// have been linked with.
 #[repr(C)]
 pub struct DeathRecipient {
     recipient: *mut sys::AIBinder_DeathRecipient,
-    callback: Box<dyn Fn() + Send + 'static>,
+    cookie: *mut c_void,
+    vtable: &'static DeathRecipientVtable,
+}
+
+struct DeathRecipientVtable {
+    cookie_incr_refcount: unsafe extern "C" fn(*mut c_void),
+    cookie_decr_refcount: unsafe extern "C" fn(*mut c_void),
 }
 
 impl DeathRecipient {
@@ -563,9 +579,9 @@
     /// associated object dies.
     pub fn new<F>(callback: F) -> DeathRecipient
     where
-        F: Fn() + Send + 'static,
+        F: Fn() + Send + Sync + 'static,
     {
-        let callback = Box::new(callback);
+        let callback: *const F = Arc::into_raw(Arc::new(callback));
         let recipient = unsafe {
             // Safety: The function pointer is a valid death recipient callback.
             //
@@ -574,34 +590,85 @@
             // no longer needed.
             sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::<F>))
         };
+        unsafe {
+            // Safety: The function pointer is a valid onUnlinked callback.
+            //
+            // All uses of linkToDeath in this file correctly increment the
+            // ref-count that this onUnlinked callback will decrement.
+            sys::AIBinder_DeathRecipient_setOnUnlinked(recipient, Some(Self::cookie_decr_refcount::<F>));
+        }
         DeathRecipient {
             recipient,
-            callback,
+            cookie: callback as *mut c_void,
+            vtable: &DeathRecipientVtable {
+                cookie_incr_refcount: Self::cookie_incr_refcount::<F>,
+                cookie_decr_refcount: Self::cookie_decr_refcount::<F>,
+            },
         }
     }
 
+    /// Increment the ref-count for the cookie and return it.
+    ///
+    /// # Safety
+    ///
+    /// The caller must handle the returned ref-count correctly.
+    unsafe fn new_cookie(&self) -> *mut c_void {
+        (self.vtable.cookie_incr_refcount)(self.cookie);
+
+        // Return a raw pointer with ownership of a ref-count
+        self.cookie
+    }
+
     /// Get the opaque cookie that identifies this death recipient.
     ///
     /// This cookie will be used to link and unlink this death recipient to a
     /// binder object and will be passed to the `binder_died` callback as an
     /// opaque userdata pointer.
     fn get_cookie(&self) -> *mut c_void {
-        &*self.callback as *const _ as *mut c_void
+        self.cookie
     }
 
     /// Callback invoked from C++ when the binder object dies.
     ///
     /// # Safety
     ///
-    /// The `cookie` parameter must have been created with the `get_cookie`
-    /// method of this object.
+    /// The `cookie` parameter must be the cookie for an Arc<F> and
+    /// the caller must hold a ref-count to it.
     unsafe extern "C" fn binder_died<F>(cookie: *mut c_void)
     where
-        F: Fn() + Send + 'static,
+        F: Fn() + Send + Sync + 'static,
     {
-        let callback = (cookie as *mut F).as_ref().unwrap();
+        let callback = (cookie as *const F).as_ref().unwrap();
         callback();
     }
+
+    /// Callback that decrements the ref-count.
+    /// This is invoked from C++ when a binder is unlinked.
+    ///
+    /// # Safety
+    ///
+    /// The `cookie` parameter must be the cookie for an Arc<F> and
+    /// the owner must give up a ref-count to it.
+    unsafe extern "C" fn cookie_decr_refcount<F>(cookie: *mut c_void)
+    where
+        F: Fn() + Send + Sync + 'static,
+    {
+        drop(Arc::from_raw(cookie as *const F));
+    }
+
+    /// Callback that increments the ref-count.
+    ///
+    /// # Safety
+    ///
+    /// The `cookie` parameter must be the cookie for an Arc<F> and
+    /// the owner must handle the created ref-count properly.
+    unsafe extern "C" fn cookie_incr_refcount<F>(cookie: *mut c_void)
+    where
+        F: Fn() + Send + Sync + 'static,
+    {
+        let arc = mem::ManuallyDrop::new(Arc::from_raw(cookie as *const F));
+        mem::forget(Arc::clone(&arc));
+    }
 }
 
 /// # Safety
@@ -627,6 +694,12 @@
             // `AIBinder_DeathRecipient_new` when `self` was created. This
             // delete method can only be called once when `self` is dropped.
             sys::AIBinder_DeathRecipient_delete(self.recipient);
+
+            // Safety: We own a ref-count to the cookie, and so does every
+            // linked binder. This call gives up our ref-count. The linked
+            // binders should already have given up their ref-count, or should
+            // do so shortly.
+            (self.vtable.cookie_decr_refcount)(self.cookie)
         }
     }
 }
diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs
index da8907d..ac9eed2 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -363,13 +363,58 @@
         );
     }
 
-    fn register_death_notification(binder: &mut SpIBinder) -> (Arc<AtomicBool>, DeathRecipient) {
+    struct Bools {
+        binder_died: Arc<AtomicBool>,
+        binder_dealloc: Arc<AtomicBool>,
+    }
+
+    impl Bools {
+        fn is_dead(&self) -> bool {
+            self.binder_died.load(Ordering::Relaxed)
+        }
+        fn assert_died(&self) {
+            assert!(
+                self.is_dead(),
+                "Did not receive death notification"
+            );
+        }
+        fn assert_dropped(&self) {
+            assert!(
+                self.binder_dealloc.load(Ordering::Relaxed),
+                "Did not dealloc death notification"
+            );
+        }
+        fn assert_not_dropped(&self) {
+            assert!(
+                !self.binder_dealloc.load(Ordering::Relaxed),
+                "Dealloc death notification too early"
+            );
+        }
+    }
+
+    fn register_death_notification(binder: &mut SpIBinder) -> (Bools, DeathRecipient) {
         let binder_died = Arc::new(AtomicBool::new(false));
+        let binder_dealloc = Arc::new(AtomicBool::new(false));
+
+        struct SetOnDrop {
+            binder_dealloc: Arc<AtomicBool>,
+        }
+        impl Drop for SetOnDrop {
+            fn drop(&mut self) {
+                self.binder_dealloc.store(true, Ordering::Relaxed);
+            }
+        }
 
         let mut death_recipient = {
             let flag = binder_died.clone();
+            let set_on_drop = SetOnDrop {
+                binder_dealloc: binder_dealloc.clone(),
+            };
             DeathRecipient::new(move || {
                 flag.store(true, Ordering::Relaxed);
+                // Force the closure to take ownership of set_on_drop. When the closure is
+                // dropped, the destructor of `set_on_drop` will run.
+                let _ = &set_on_drop;
             })
         };
 
@@ -377,7 +422,12 @@
             .link_to_death(&mut death_recipient)
             .expect("link_to_death failed");
 
-        (binder_died, death_recipient)
+        let bools = Bools {
+            binder_died,
+            binder_dealloc,
+        };
+
+        (bools, death_recipient)
     }
 
     /// Killing a remote service should unregister the service and trigger
@@ -390,7 +440,7 @@
         let service_process = ScopedServiceProcess::new(service_name);
         let mut remote = binder::get_service(service_name).expect("Could not retrieve service");
 
-        let (binder_died, _recipient) = register_death_notification(&mut remote);
+        let (bools, recipient) = register_death_notification(&mut remote);
 
         drop(service_process);
         remote
@@ -400,10 +450,12 @@
         // Pause to ensure any death notifications get delivered
         thread::sleep(Duration::from_secs(1));
 
-        assert!(
-            binder_died.load(Ordering::Relaxed),
-            "Did not receive death notification"
-        );
+        bools.assert_died();
+        bools.assert_not_dropped();
+
+        drop(recipient);
+
+        bools.assert_dropped();
     }
 
     /// Test unregistering death notifications.
@@ -415,7 +467,7 @@
         let service_process = ScopedServiceProcess::new(service_name);
         let mut remote = binder::get_service(service_name).expect("Could not retrieve service");
 
-        let (binder_died, mut recipient) = register_death_notification(&mut remote);
+        let (bools, mut recipient) = register_death_notification(&mut remote);
 
         remote
             .unlink_to_death(&mut recipient)
@@ -430,9 +482,13 @@
         thread::sleep(Duration::from_secs(1));
 
         assert!(
-            !binder_died.load(Ordering::Relaxed),
+            !bools.is_dead(),
             "Received unexpected death notification after unlinking",
         );
+
+        bools.assert_not_dropped();
+        drop(recipient);
+        bools.assert_dropped();
     }
 
     /// Dropping a remote handle should unregister any death notifications.
@@ -444,7 +500,7 @@
         let service_process = ScopedServiceProcess::new(service_name);
         let mut remote = binder::get_service(service_name).expect("Could not retrieve service");
 
-        let (binder_died, _recipient) = register_death_notification(&mut remote);
+        let (bools, recipient) = register_death_notification(&mut remote);
 
         // This should automatically unregister our death notification.
         drop(remote);
@@ -457,9 +513,13 @@
         // We dropped the remote handle, so we should not receive the death
         // notification when the remote process dies here.
         assert!(
-            !binder_died.load(Ordering::Relaxed),
+            !bools.is_dead(),
             "Received unexpected death notification after dropping remote handle"
         );
+
+        bools.assert_not_dropped();
+        drop(recipient);
+        bools.assert_dropped();
     }
 
     /// Test IBinder interface methods not exercised elsewhere.