Merge "libbinder_ndk: integration test for linkToDeath" am: 3964935f74 am: 358d8125a0
am: 80d5ae49e2

Change-Id: I10793034b185d30c47f07222964bce916905a3df
diff --git a/libs/binder/ndk/runtests.sh b/libs/binder/ndk/runtests.sh
index 2257eb2..a0c49fb 100755
--- a/libs/binder/ndk/runtests.sh
+++ b/libs/binder/ndk/runtests.sh
@@ -22,12 +22,13 @@
 set -ex
 
 function run_libbinder_ndk_test() {
-	adb shell /data/nativetest64/libbinder_ndk_test_server/libbinder_ndk_test_server &
-	local pid=$!
-	trap "kill $pid" ERR
-	adb shell /data/nativetest64/libbinder_ndk_test_client/libbinder_ndk_test_client
-	trap '' ERR
-	kill $pid
+    adb shell /data/nativetest64/libbinder_ndk_test_server/libbinder_ndk_test_server &
+
+    # avoid getService 1s delay for most runs, non-critical
+    sleep 0.1
+
+    adb shell /data/nativetest64/libbinder_ndk_test_client/libbinder_ndk_test_client; \
+        adb shell killall libbinder_ndk_test_server
 }
 
 [ "$1" != "--skip-build" ] && $ANDROID_BUILD_TOP/build/soong/soong_ui.bash --make-mode \
@@ -40,4 +41,5 @@
 # very simple unit tests, tests things outside of the NDK as well
 run_libbinder_ndk_test
 
-atest android.binder.cts.NdkBinderTest
+# CTS tests (much more comprehensive, new tests should ideally go here)
+atest android.binder.cts
diff --git a/libs/binder/ndk/test/Android.bp b/libs/binder/ndk/test/Android.bp
index 8e40a01..b29b6e7 100644
--- a/libs/binder/ndk/test/Android.bp
+++ b/libs/binder/ndk/test/Android.bp
@@ -14,9 +14,6 @@
  * limitations under the License.
  */
 
-// This test is a unit test of the low-level API that is presented here.
-// Actual users should use AIDL to generate these complicated stubs.
-
 cc_defaults {
     name: "test_libbinder_ndk_defaults",
     shared_libs: [
@@ -55,6 +52,9 @@
     ],
 }
 
+// This test is a unit test of the low-level API that is presented here,
+// specifically the parts which are outside of the NDK. Actual users should
+// also instead use AIDL to generate these stubs. See android.binder.cts.
 cc_test {
     name: "libbinder_ndk_test_client",
     defaults: ["test_libbinder_ndk_test_defaults"],
diff --git a/libs/binder/ndk/test/iface.cpp b/libs/binder/ndk/test/iface.cpp
index d46fde8..6ef964e 100644
--- a/libs/binder/ndk/test/iface.cpp
+++ b/libs/binder/ndk/test/iface.cpp
@@ -18,10 +18,13 @@
 #include <android/binder_manager.h>
 #include <iface/iface.h>
 
+#include <android/binder_auto_utils.h>
+
 using ::android::sp;
 using ::android::wp;
 
 const char* IFoo::kSomeInstanceName = "libbinder_ndk-test-IFoo";
+const char* IFoo::kInstanceNameToDieFor = "libbinder_ndk-test-IFoo-to-die";
 const char* kIFooDescriptor = "my-special-IFoo-class";
 
 struct IFoo_Class_Data {
@@ -49,12 +52,18 @@
     switch (code) {
         case IFoo::DOFOO: {
             int32_t valueIn;
+            int32_t valueOut;
             stat = AParcel_readInt32(in, &valueIn);
             if (stat != STATUS_OK) break;
-            int32_t valueOut = foo->doubleNumber(valueIn);
+            stat = foo->doubleNumber(valueIn, &valueOut);
+            if (stat != STATUS_OK) break;
             stat = AParcel_writeInt32(out, valueOut);
             break;
         }
+        case IFoo::DIE: {
+            stat = foo->die();
+            break;
+        }
     }
 
     return stat;
@@ -68,22 +77,36 @@
     BpFoo(AIBinder* binder) : mBinder(binder) {}
     virtual ~BpFoo() { AIBinder_decStrong(mBinder); }
 
-    virtual int32_t doubleNumber(int32_t in) {
+    virtual binder_status_t doubleNumber(int32_t in, int32_t* out) {
+        binder_status_t stat = STATUS_OK;
+
         AParcel* parcelIn;
-        CHECK(STATUS_OK == AIBinder_prepareTransaction(mBinder, &parcelIn));
+        stat = AIBinder_prepareTransaction(mBinder, &parcelIn);
+        if (stat != STATUS_OK) return stat;
 
-        CHECK(STATUS_OK == AParcel_writeInt32(parcelIn, in));
+        stat = AParcel_writeInt32(parcelIn, in);
+        if (stat != STATUS_OK) return stat;
 
-        AParcel* parcelOut;
-        CHECK(STATUS_OK ==
-              AIBinder_transact(mBinder, IFoo::DOFOO, &parcelIn, &parcelOut, 0 /*flags*/));
+        ::ndk::ScopedAParcel parcelOut;
+        stat = AIBinder_transact(mBinder, IFoo::DOFOO, &parcelIn, parcelOut.getR(), 0 /*flags*/);
+        if (stat != STATUS_OK) return stat;
 
-        int32_t out;
-        CHECK(STATUS_OK == AParcel_readInt32(parcelOut, &out));
+        stat = AParcel_readInt32(parcelOut.get(), out);
+        if (stat != STATUS_OK) return stat;
 
-        AParcel_delete(parcelOut);
+        return stat;
+    }
 
-        return out;
+    virtual binder_status_t die() {
+        binder_status_t stat = STATUS_OK;
+
+        AParcel* parcelIn;
+        stat = AIBinder_prepareTransaction(mBinder, &parcelIn);
+
+        ::ndk::ScopedAParcel parcelOut;
+        stat = AIBinder_transact(mBinder, IFoo::DIE, &parcelIn, parcelOut.getR(), 0 /*flags*/);
+
+        return stat;
     }
 
    private:
@@ -117,7 +140,7 @@
     return status;
 }
 
-sp<IFoo> IFoo::getService(const char* instance) {
+sp<IFoo> IFoo::getService(const char* instance, AIBinder** outBinder) {
     AIBinder* binder = AServiceManager_getService(instance);  // maybe nullptr
     if (binder == nullptr) {
         return nullptr;
@@ -128,6 +151,11 @@
         return nullptr;
     }
 
+    if (outBinder != nullptr) {
+        AIBinder_incStrong(binder);
+        *outBinder = binder;
+    }
+
     if (AIBinder_isRemote(binder)) {
         sp<IFoo> ret = new BpFoo(binder);  // takes ownership of binder
         return ret;
@@ -143,7 +171,6 @@
     CHECK(held == binder);
     AIBinder_decStrong(held);
 
-    // IFoo only keeps a weak reference to AIBinder, so we can drop this
     AIBinder_decStrong(binder);
     return ret;
 }
diff --git a/libs/binder/ndk/test/include/iface/iface.h b/libs/binder/ndk/test/include/iface/iface.h
index 25f5188..cdf5493 100644
--- a/libs/binder/ndk/test/include/iface/iface.h
+++ b/libs/binder/ndk/test/include/iface/iface.h
@@ -19,22 +19,33 @@
 #include <android/binder_ibinder.h>
 #include <utils/RefBase.h>
 
+// warning: it is recommended to use AIDL output instead of this. binder_ibinder_utils.h and some of
+// the other niceties make sure that, for instance, binder proxies are always the same. They also
+// don't use internal Android APIs like refbase which are used here only for convenience.
+
 class IFoo : public virtual ::android::RefBase {
    public:
     static const char* kSomeInstanceName;
+    static const char* kInstanceNameToDieFor;
+
     static AIBinder_Class* kClass;
 
     // Takes ownership of IFoo
     binder_status_t addService(const char* instance);
-    static ::android::sp<IFoo> getService(const char* instance);
+    static ::android::sp<IFoo> getService(const char* instance, AIBinder** outBinder = nullptr);
 
     enum Call {
         DOFOO = FIRST_CALL_TRANSACTION + 0,
+        DIE = FIRST_CALL_TRANSACTION + 1,
     };
 
     virtual ~IFoo();
-    virtual int32_t doubleNumber(int32_t in) = 0;
+
+    virtual binder_status_t doubleNumber(int32_t in, int32_t* out) = 0;
+    virtual binder_status_t die() = 0;
 
    private:
-    AIBinder_Weak* mWeakBinder = nullptr;  // maybe owns AIBinder
+    // this variable is only when IFoo is local (since this test combines 'IFoo' and 'BnFoo'), not
+    // for BpFoo.
+    AIBinder_Weak* mWeakBinder = nullptr;
 };
diff --git a/libs/binder/ndk/test/main_client.cpp b/libs/binder/ndk/test/main_client.cpp
index 507daa2..c159d71 100644
--- a/libs/binder/ndk/test/main_client.cpp
+++ b/libs/binder/ndk/test/main_client.cpp
@@ -21,6 +21,10 @@
 #include <gtest/gtest.h>
 #include <iface/iface.h>
 
+#include <chrono>
+#include <condition_variable>
+#include <mutex>
+
 using ::android::sp;
 
 constexpr char kExistingNonNdkService[] = "SurfaceFlinger";
@@ -34,7 +38,47 @@
 TEST(NdkBinder, DoubleNumber) {
     sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);
     ASSERT_NE(foo, nullptr);
-    EXPECT_EQ(2, foo->doubleNumber(1));
+
+    int32_t out;
+    EXPECT_EQ(STATUS_OK, foo->doubleNumber(1, &out));
+    EXPECT_EQ(2, out);
+}
+
+void LambdaOnDeath(void* cookie) {
+    auto onDeath = static_cast<std::function<void(void)>*>(cookie);
+    (*onDeath)();
+};
+TEST(NdkBinder, DeathRecipient) {
+    using namespace std::chrono_literals;
+
+    AIBinder* binder;
+    sp<IFoo> foo = IFoo::getService(IFoo::kInstanceNameToDieFor, &binder);
+    ASSERT_NE(nullptr, foo.get());
+    ASSERT_NE(nullptr, binder);
+
+    std::mutex deathMutex;
+    std::condition_variable deathCv;
+    bool deathRecieved = false;
+
+    std::function<void(void)> onDeath = [&] {
+        std::cerr << "Binder died (as requested)." << std::endl;
+        deathRecieved = true;
+        deathCv.notify_one();
+    };
+
+    AIBinder_DeathRecipient* recipient = AIBinder_DeathRecipient_new(LambdaOnDeath);
+
+    EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(&onDeath)));
+
+    // the binder driver should return this if the service dies during the transaction
+    EXPECT_EQ(STATUS_DEAD_OBJECT, foo->die());
+
+    std::unique_lock<std::mutex> lock(deathMutex);
+    EXPECT_TRUE(deathCv.wait_for(lock, 1s, [&] { return deathRecieved; }));
+    EXPECT_TRUE(deathRecieved);
+
+    AIBinder_DeathRecipient_delete(recipient);
+    AIBinder_decStrong(binder);
 }
 
 TEST(NdkBinder, RetrieveNonNdkService) {
@@ -52,9 +96,6 @@
 }
 
 TEST(NdkBinder, LinkToDeath) {
-    ABinderProcess_setThreadPoolMaxThreadCount(1);  // to recieve death notifications
-    ABinderProcess_startThreadPool();
-
     AIBinder* binder = AServiceManager_getService(kExistingNonNdkService);
     ASSERT_NE(nullptr, binder);
 
@@ -72,9 +113,14 @@
 }
 
 class MyTestFoo : public IFoo {
-    int32_t doubleNumber(int32_t in) override {
-        LOG(INFO) << "doubleNumber " << in;
-        return 2 * in;
+    binder_status_t doubleNumber(int32_t in, int32_t* out) override {
+        *out = 2 * in;
+        LOG(INFO) << "doubleNumber (" << in << ") => " << *out;
+        return STATUS_OK;
+    }
+    binder_status_t die() override {
+        ADD_FAILURE() << "die called on local instance";
+        return STATUS_OK;
     }
 };
 
@@ -87,7 +133,9 @@
     sp<IFoo> getFoo = IFoo::getService(kInstanceName);
     EXPECT_EQ(foo.get(), getFoo.get());
 
-    EXPECT_EQ(2, getFoo->doubleNumber(1));
+    int32_t out;
+    EXPECT_EQ(STATUS_OK, getFoo->doubleNumber(1, &out));
+    EXPECT_EQ(2, out);
 }
 
 TEST(NdkBinder, EqualityOfRemoteBinderPointer) {
@@ -132,6 +180,15 @@
     EXPECT_EQ(IFoo::getService(kInstanceName1), IFoo::getService(kInstanceName2));
 }
 
+int main(int argc, char* argv[]) {
+    ::testing::InitGoogleTest(&argc, argv);
+
+    ABinderProcess_setThreadPoolMaxThreadCount(1);  // to recieve death notifications/callbacks
+    ABinderProcess_startThreadPool();
+
+    return RUN_ALL_TESTS();
+}
+
 #include <android/binder_auto_utils.h>
 #include <android/binder_interface_utils.h>
 #include <android/binder_parcel_utils.h>
diff --git a/libs/binder/ndk/test/main_server.cpp b/libs/binder/ndk/test/main_server.cpp
index 0718a69..a6e17e8 100644
--- a/libs/binder/ndk/test/main_server.cpp
+++ b/libs/binder/ndk/test/main_server.cpp
@@ -21,23 +21,37 @@
 using ::android::sp;
 
 class MyFoo : public IFoo {
-    int32_t doubleNumber(int32_t in) override {
-        LOG(INFO) << "doubling " << in;
-        return 2 * in;
+    binder_status_t doubleNumber(int32_t in, int32_t* out) override {
+        *out = 2 * in;
+        LOG(INFO) << "doubleNumber (" << in << ") => " << *out;
+        return STATUS_OK;
+    }
+
+    binder_status_t die() override {
+        LOG(FATAL) << "IFoo::die called!";
+        return STATUS_UNKNOWN_ERROR;
     }
 };
 
-int main() {
+int service(const char* instance) {
     ABinderProcess_setThreadPoolMaxThreadCount(0);
 
     // Strong reference to MyFoo kept by service manager.
-    binder_status_t status = (new MyFoo)->addService(IFoo::kSomeInstanceName);
+    binder_status_t status = (new MyFoo)->addService(instance);
 
     if (status != STATUS_OK) {
-        LOG(FATAL) << "Could not register: " << status;
+        LOG(FATAL) << "Could not register: " << status << " " << instance;
     }
 
     ABinderProcess_joinThreadPool();
 
-    return 1;
+    return 1;  // should not return
+}
+
+int main() {
+    if (fork() == 0) {
+        return service(IFoo::kInstanceNameToDieFor);
+    }
+
+    return service(IFoo::kSomeInstanceName);
 }