Merge "libbinder_ndk: stress test for retrieving binders"
diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp
index 07f5778..1c2f9b4 100644
--- a/libs/binder/ServiceManagerHost.cpp
+++ b/libs/binder/ServiceManagerHost.cpp
@@ -167,9 +167,12 @@
         ALOGE("RpcSession::getRootObject returns nullptr");
         return nullptr;
     }
-    binder->attachObject(kDeviceServiceExtraId,
-                         static_cast<void*>(new CommandResult(std::move(*result))), nullptr,
-                         &cleanupCommandResult);
+
+    LOG_ALWAYS_FATAL_IF(
+            nullptr !=
+            binder->attachObject(kDeviceServiceExtraId,
+                                 static_cast<void*>(new CommandResult(std::move(*result))), nullptr,
+                                 &cleanupCommandResult));
     return binder;
 }
 
diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
index 748fa72..269b867 100644
--- a/libs/binder/TEST_MAPPING
+++ b/libs/binder/TEST_MAPPING
@@ -16,6 +16,9 @@
       "name": "binderDriverInterfaceTest"
     },
     {
+      "name": "binderHostDeviceTest"
+    },
+    {
       "name": "binderTextOutputTest"
     },
     {
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index c484d83..43fc5ff 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -259,21 +259,20 @@
      * but calls from different threads are allowed to be interleaved.
      *
      * This returns the object which is already attached. If this returns a
-     * non-null value, it means that attachObject failed. TODO(b/192023359):
-     * remove logs and add [[nodiscard]]
+     * non-null value, it means that attachObject failed (a given objectID can
+     * only be used once).
      */
-    virtual void* attachObject(const void* objectID, void* object, void* cleanupCookie,
-                               object_cleanup_func func) = 0;
+    [[nodiscard]] virtual void* attachObject(const void* objectID, void* object,
+                                             void* cleanupCookie, object_cleanup_func func) = 0;
     /**
      * Returns object attached with attachObject.
      */
-    virtual void*           findObject(const void* objectID) const = 0;
+    [[nodiscard]] virtual void* findObject(const void* objectID) const = 0;
     /**
      * Returns object attached with attachObject, and detaches it. This does not
-     * delete the object. This is equivalent to using attachObject to attach a null
-     * object.
+     * delete the object.
      */
-    virtual void* detachObject(const void* objectID) = 0;
+    [[nodiscard]] virtual void* detachObject(const void* objectID) = 0;
 
     /**
      * Use the lock that this binder contains internally. For instance, this can
diff --git a/libs/binder/include/binder/ParcelableHolder.h b/libs/binder/include/binder/ParcelableHolder.h
index 9e4475c..42c85f9 100644
--- a/libs/binder/include/binder/ParcelableHolder.h
+++ b/libs/binder/include/binder/ParcelableHolder.h
@@ -42,6 +42,7 @@
         }
         mStability = other.mStability;
     }
+    ParcelableHolder(ParcelableHolder&& other) = default;
 
     status_t writeToParcel(Parcel* parcel) const override;
     status_t readFromParcel(const Parcel* parcel) override;
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 65214bd..2abb6f7 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -47,7 +47,8 @@
 void clean(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */};
 
 static void attach(const sp<IBinder>& binder) {
-    binder->attachObject(kId, kValue, nullptr /*cookie*/, clean);
+    // can only attach once
+    CHECK_EQ(nullptr, binder->attachObject(kId, kValue, nullptr /*cookie*/, clean));
 }
 static bool has(const sp<IBinder>& binder) {
     return binder != nullptr && binder->findObject(kId) == kValue;
diff --git a/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h b/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
index 2277148..aa3b978 100644
--- a/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
@@ -46,7 +46,7 @@
     AParcelableHolder() = delete;
     explicit AParcelableHolder(parcelable_stability_t stability)
         : mParcel(AParcel_create()), mStability(stability) {}
-
+    AParcelableHolder(AParcelableHolder&& other) = default;
     virtual ~AParcelableHolder() = default;
 
     binder_status_t writeToParcel(AParcel* parcel) const {
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 565f88e..24afcf6 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -348,3 +348,49 @@
         },
     },
 }
+
+cc_test_host {
+    name: "binderHostDeviceTest",
+    defaults: ["binder_test_defaults"],
+    srcs: ["binderHostDeviceTest.cpp"],
+    test_config: "binderHostDeviceTest.xml",
+    shared_libs: [
+        "libbase",
+        "libbinder",
+        "liblog",
+        "libutils",
+    ],
+    static_libs: [
+        "libgmock",
+    ],
+    target_required: [
+        "binderHostDeviceTestService",
+    ],
+    test_suites: ["general-tests"],
+    target: {
+        darwin: {
+            enabled: false,
+        },
+    },
+    test_options: {
+        unit_test: false,
+    },
+}
+
+cc_test {
+    name: "binderHostDeviceTestService",
+    // The binary is named differently from the module so that PushFilePreparer pushes the binary
+    // directly, not the test module directory.
+    stem: "binderHostDeviceTest-service",
+    defaults: ["binder_test_defaults"],
+    gtest: false,
+    auto_gen_config: false,
+    srcs: ["binderHostDeviceTestService.cpp"],
+    shared_libs: [
+        "libbase",
+        "libbinder",
+        "liblog",
+        "libutils",
+    ],
+    test_suites: ["general-tests"],
+}
diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp
new file mode 100644
index 0000000..5dd9212
--- /dev/null
+++ b/libs/binder/tests/binderHostDeviceTest.cpp
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Integration test for servicedispatcher + adb forward. Requires ADB.
+
+#include <stdlib.h>
+
+#include <vector>
+
+#include <android-base/parsebool.h>
+#include <android-base/result-gmock.h>
+#include <android-base/strings.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <binder/IServiceManager.h>
+#include <binder/RpcSession.h>
+
+#include "../UtilsHost.h"
+
+using ::android::setDefaultServiceManager;
+using ::android::base::EndsWith;
+using ::android::base::Join;
+using ::android::base::ParseBool;
+using ::android::base::ParseBoolResult;
+using ::android::base::Split;
+using ::android::base::StartsWith;
+using ::android::base::StringReplace;
+using ::android::base::Trim;
+using ::android::base::testing::Ok;
+using ::std::chrono_literals::operator""ms;
+using ::std::string_literals::operator""s;
+using ::testing::AllOf;
+using ::testing::Contains;
+using ::testing::ContainsRegex;
+using ::testing::ExplainMatchResult;
+using ::testing::InitGoogleMock;
+
+namespace android {
+
+namespace {
+
+constexpr const char* kServiceBinary = "/data/local/tmp/binderHostDeviceTest-service";
+constexpr const char* kServiceName = "binderHostDeviceTestService";
+constexpr const char* kDescriptor = "android.binderHostDeviceTestService";
+
+// e.g. EXPECT_THAT(expr, StatusEq(OK)) << "additional message";
+MATCHER_P(StatusEq, expected, (negation ? "not " : "") + statusToString(expected)) {
+    *result_listener << statusToString(arg);
+    return expected == arg;
+}
+
+void initHostRpcServiceManagerOnce() {
+    static std::once_flag gSmOnce;
+    std::call_once(gSmOnce, [] { setDefaultServiceManager(createRpcDelegateServiceManager()); });
+}
+
+// Test for host service manager.
+class HostDeviceTest : public ::testing::Test {
+public:
+    void SetUp() override {
+        auto debuggableResult = execute(Split("adb shell getprop ro.debuggable", " "), nullptr);
+        ASSERT_THAT(debuggableResult, Ok());
+        ASSERT_EQ(0, debuggableResult->exitCode) << *debuggableResult;
+        auto debuggableBool = ParseBool(Trim(debuggableResult->stdout));
+        ASSERT_NE(ParseBoolResult::kError, debuggableBool) << Trim(debuggableResult->stdout);
+        if (debuggableBool == ParseBoolResult::kFalse) {
+            GTEST_SKIP() << "ro.debuggable=" << Trim(debuggableResult->stdout);
+        }
+
+        auto lsResult = execute(Split("adb shell which servicedispatcher", " "), nullptr);
+        ASSERT_THAT(lsResult, Ok());
+        if (lsResult->exitCode != 0) {
+            GTEST_SKIP() << "b/182914638: until feature is fully enabled, skip test on devices "
+                            "without servicedispatcher";
+        }
+
+        initHostRpcServiceManagerOnce();
+        ASSERT_NE(nullptr, defaultServiceManager()) << "No defaultServiceManager() over RPC";
+
+        auto service = execute({"adb", "shell", kServiceBinary, kServiceName, kDescriptor},
+                               &CommandResult::stdoutEndsWithNewLine);
+        ASSERT_THAT(service, Ok());
+        ASSERT_EQ(std::nullopt, service->exitCode) << *service;
+        mService = std::move(*service);
+    }
+    void TearDown() override { mService.reset(); }
+
+    [[nodiscard]] static sp<IBinder> get(unsigned int hostPort) {
+        auto rpcSession = RpcSession::make();
+        if (!rpcSession->setupInetClient("127.0.0.1", hostPort)) {
+            ADD_FAILURE() << "Failed to setupInetClient on " << hostPort;
+            return nullptr;
+        }
+        return rpcSession->getRootObject();
+    }
+
+private:
+    std::optional<CommandResult> mService;
+};
+
+TEST_F(HostDeviceTest, List) {
+    auto sm = defaultServiceManager();
+
+    auto services = sm->listServices();
+    ASSERT_THAT(services, Contains(String16(kServiceName)));
+}
+
+TEST_F(HostDeviceTest, CheckService) {
+    auto sm = defaultServiceManager();
+
+    auto rpcBinder = sm->checkService(String16(kServiceName));
+    ASSERT_NE(nullptr, rpcBinder);
+
+    EXPECT_THAT(rpcBinder->pingBinder(), StatusEq(OK));
+    EXPECT_EQ(String16(kDescriptor), rpcBinder->getInterfaceDescriptor());
+}
+
+TEST_F(HostDeviceTest, GetService) {
+    auto sm = defaultServiceManager();
+
+    auto rpcBinder = sm->getService(String16(kServiceName));
+    ASSERT_NE(nullptr, rpcBinder);
+
+    EXPECT_THAT(rpcBinder->pingBinder(), StatusEq(OK));
+    EXPECT_EQ(String16(kDescriptor), rpcBinder->getInterfaceDescriptor());
+}
+
+TEST_F(HostDeviceTest, WaitForService) {
+    auto sm = defaultServiceManager();
+
+    auto rpcBinder = sm->waitForService(String16(kServiceName));
+    ASSERT_NE(nullptr, rpcBinder);
+
+    EXPECT_THAT(rpcBinder->pingBinder(), StatusEq(OK));
+    EXPECT_EQ(String16(kDescriptor), rpcBinder->getInterfaceDescriptor());
+}
+
+TEST_F(HostDeviceTest, TenClients) {
+    auto sm = defaultServiceManager();
+
+    auto threadFn = [&] {
+        auto rpcBinder = sm->checkService(String16(kServiceName));
+        ASSERT_NE(nullptr, rpcBinder);
+
+        EXPECT_THAT(rpcBinder->pingBinder(), StatusEq(OK));
+        EXPECT_EQ(String16(kDescriptor), rpcBinder->getInterfaceDescriptor());
+    };
+
+    std::vector<std::thread> threads;
+    for (size_t i = 0; i < 10; ++i) threads.emplace_back(threadFn);
+    for (auto& thread : threads) thread.join();
+}
+
+} // namespace
+
+} // namespace android
diff --git a/libs/binder/tests/binderHostDeviceTest.xml b/libs/binder/tests/binderHostDeviceTest.xml
new file mode 100644
index 0000000..250ed3a
--- /dev/null
+++ b/libs/binder/tests/binderHostDeviceTest.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2021 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+<configuration description="Runs binderHostDeviceTest.">
+    <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer"/>
+    <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+        <option name="cleanup" value="true" />
+        <option name="push-file" key="binderHostDeviceTest-service"
+                value="/data/local/tmp/binderHostDeviceTest-service"/>
+    </target_preparer>
+
+    <test class="com.android.tradefed.testtype.binary.ExecutableHostTest">
+        <option name="binary" value="binderHostDeviceTest"/>
+    </test>
+</configuration>
diff --git a/libs/binder/tests/binderHostDeviceTestService.cpp b/libs/binder/tests/binderHostDeviceTestService.cpp
new file mode 100644
index 0000000..6ddd2e7
--- /dev/null
+++ b/libs/binder/tests/binderHostDeviceTestService.cpp
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <sysexits.h>
+
+#include <android-base/logging.h>
+#include <binder/IBinder.h>
+#include <binder/IPCThreadState.h>
+#include <binder/IServiceManager.h>
+#include <binder/ProcessState.h>
+
+namespace {
+class Service : public android::BBinder {
+public:
+    Service(std::string_view descriptor) : mDescriptor(descriptor.data(), descriptor.size()) {}
+    const android::String16& getInterfaceDescriptor() const override { return mDescriptor; }
+
+private:
+    android::String16 mDescriptor;
+};
+} // namespace
+
+int main(int argc, char** argv) {
+    if (argc != 3) {
+        std::cerr << "usage: " << argv[0] << " <service-name> <interface-descriptor>" << std::endl;
+        return EX_USAGE;
+    }
+    auto name = argv[1];
+    auto descriptor = argv[2];
+
+    auto sm = android::defaultServiceManager();
+    CHECK(sm != nullptr);
+    auto service = android::sp<Service>::make(descriptor);
+    auto status = sm->addService(android::String16(name), service);
+    CHECK_EQ(android::OK, status) << android::statusToString(status);
+    std::cout << "running..." << std::endl;
+    android::ProcessState::self()->startThreadPool();
+    android::IPCThreadState::self()->joinThreadPool();
+    LOG(ERROR) << "joinThreadPool exits";
+    return EX_SOFTWARE;
+}
diff --git a/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h
index 626b758..4a0aeba 100644
--- a/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/IBinderFuzzFunctions.h
@@ -62,20 +62,22 @@
              object = fdp->ConsumeIntegral<uint32_t>();
              cleanup_cookie = fdp->ConsumeIntegral<uint32_t>();
              IBinder::object_cleanup_func func = IBinder::object_cleanup_func();
-             ibinder->attachObject(fdp->ConsumeBool() ? reinterpret_cast<void*>(&objectID)
-                                                      : nullptr,
-                                   fdp->ConsumeBool() ? reinterpret_cast<void*>(&object) : nullptr,
-                                   fdp->ConsumeBool() ? reinterpret_cast<void*>(&cleanup_cookie)
-                                                      : nullptr,
-                                   func);
+             (void)ibinder->attachObject(fdp->ConsumeBool() ? reinterpret_cast<void*>(&objectID)
+                                                            : nullptr,
+                                         fdp->ConsumeBool() ? reinterpret_cast<void*>(&object)
+                                                            : nullptr,
+                                         fdp->ConsumeBool()
+                                                 ? reinterpret_cast<void*>(&cleanup_cookie)
+                                                 : nullptr,
+                                         func);
          },
          [](FuzzedDataProvider* fdp, IBinder* ibinder) -> void {
              uint32_t id = fdp->ConsumeIntegral<uint32_t>();
-             ibinder->findObject(reinterpret_cast<void*>(&id));
+             (void)ibinder->findObject(reinterpret_cast<void*>(&id));
          },
          [](FuzzedDataProvider* fdp, IBinder* ibinder) -> void {
              uint32_t id = fdp->ConsumeIntegral<uint32_t>();
-             ibinder->detachObject(reinterpret_cast<void*>(&id));
+             (void)ibinder->detachObject(reinterpret_cast<void*>(&id));
          },
          [](FuzzedDataProvider* fdp, IBinder* ibinder) -> void {
              uint32_t code = fdp->ConsumeIntegral<uint32_t>();