Implement setListener/clearListener internal API

- Add binder death handler
- Add tests

Bug: 182608968
Test: atest cartelemetryd_impl_test
Change-Id: I1c71a12d5feb262f50df45f03463805a37015c47
Merged-In: I1c71a12d5feb262f50df45f03463805a37015c47
(cherry picked from commit 31af79f6453846dc07dd5002922f0f1a3ac1bc82)

11 files changed, 294 insertions, 20 deletions

diff --git a/cpp/telemetry/Android.bp b/cpp/telemetry/Android.bp
index 4640be9df2..c47f75114f 100644
--- a/cpp/telemetry/Android.bp
+++ b/cpp/telemetry/Android.bp
@@ -50,6 +50,7 @@ cc_library {
         "src/RingBuffer.cpp",
         "src/TelemetryServer.cpp",
         "src/CarTelemetryInternalImpl.cpp",
+        "src/binderutils/BinderDeathRecipient.cpp",
     ],
     // Allow dependencies to use header files.
     export_include_dirs: [
@@ -65,6 +66,7 @@ cc_test {
     test_suites: ["general-tests"],
     srcs: [
         "tests/CarTelemetryImplTest.cpp",
+        "tests/CarTelemetryInternalImplTest.cpp",
         "tests/RingBufferTest.cpp",
     ],
     // Statically link only in tests, for portability reason.
diff --git a/cpp/telemetry/aidl/android/automotive/telemetry/internal/ICarTelemetryInternal.aidl b/cpp/telemetry/aidl/android/automotive/telemetry/internal/ICarTelemetryInternal.aidl
index 9f7c834f97..b8938ffd1c 100644
--- a/cpp/telemetry/aidl/android/automotive/telemetry/internal/ICarTelemetryInternal.aidl
+++ b/cpp/telemetry/aidl/android/automotive/telemetry/internal/ICarTelemetryInternal.aidl
@@ -27,7 +27,7 @@ interface ICarTelemetryInternal {
    * start pushing them to the listener. There can be only a single registered listener at a time.
    *
    * @param listener the only listener.
-   * @throws IllegalStateException if someone is already registered.
+   * @throws IllegalStateException if someone is already registered or the listener is dead.
    */
   void setListener(in ICarDataListener listener);
 
diff --git a/cpp/telemetry/src/CarTelemetryImpl.h b/cpp/telemetry/src/CarTelemetryImpl.h
index 173a472a55..593cc5f38d 100644
--- a/cpp/telemetry/src/CarTelemetryImpl.h
+++ b/cpp/telemetry/src/CarTelemetryImpl.h
@@ -17,14 +17,13 @@
 #ifndef CPP_TELEMETRY_SRC_CARTELEMETRYIMPL_H_
 #define CPP_TELEMETRY_SRC_CARTELEMETRYIMPL_H_
 
+#include "RingBuffer.h"
+
 #include <android/frameworks/automotive/telemetry/BnCarTelemetry.h>
 #include <android/frameworks/automotive/telemetry/CarData.h>
 #include <utils/String16.h>
 #include <utils/Vector.h>
 
-#include <RingBuffer.h>
-
-#include <memory>
 #include <vector>
 
 namespace android {
diff --git a/cpp/telemetry/src/CarTelemetryInternalImpl.cpp b/cpp/telemetry/src/CarTelemetryInternalImpl.cpp
index ab9def44ec..d13a09f75f 100644
--- a/cpp/telemetry/src/CarTelemetryInternalImpl.cpp
+++ b/cpp/telemetry/src/CarTelemetryInternalImpl.cpp
@@ -19,27 +19,62 @@
 #include "BufferedCarData.h"
 
 #include <android-base/logging.h>
+#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
+#include <android/automotive/telemetry/internal/BnCarDataListener.h>
 #include <android/automotive/telemetry/internal/CarDataInternal.h>
 #include <android/automotive/telemetry/internal/ICarDataListener.h>
+#include <binder/IPCThreadState.h>
 
 namespace android {
 namespace automotive {
 namespace telemetry {
 
 using ::android::sp;
+using ::android::automotive::telemetry::internal::BnCarDataListener;
 using ::android::automotive::telemetry::internal::CarDataInternal;
 using ::android::automotive::telemetry::internal::ICarDataListener;
+using ::android::base::StringPrintf;
 using ::android::binder::Status;
 
-CarTelemetryInternalImpl::CarTelemetryInternalImpl(RingBuffer* buffer) : mRingBuffer(buffer) {}
+CarTelemetryInternalImpl::CarTelemetryInternalImpl(RingBuffer* buffer) : mRingBuffer(buffer) {
+    mBinderDeathRecipient = new BinderDeathRecipient(
+            [this](const wp<android::IBinder>& binder) { listenerBinderDied(binder); });
+}
 
 Status CarTelemetryInternalImpl::setListener(const sp<ICarDataListener>& listener) {
-    // TODO(b/182608968): implement
+    const std::scoped_lock<std::mutex> lock(mMutex);
+
+    if (mCarDataListener != nullptr) {
+        return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE,
+                                         "CarDataListener is already set.");
+    }
+
+    sp<IBinder> binder = BnCarDataListener::asBinder(listener);
+    status_t status = binder->linkToDeath(mBinderDeathRecipient);
+    if (status != android::OK) {
+        pid_t callingPid = IPCThreadState::self()->getCallingPid();
+        uid_t callingUid = IPCThreadState::self()->getCallingUid();
+        std::string errorStr = StringPrintf("The given callback(pid: %d, uid: %d) is dead",
+                                            callingPid, callingUid);
+        return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE, errorStr.c_str());
+    }
+
+    mCarDataListener = listener;
     return Status::ok();
 }
 
 Status CarTelemetryInternalImpl::clearListener() {
-    // TODO(b/182608968): implement
+    const std::scoped_lock<std::mutex> lock(mMutex);
+    if (mCarDataListener == nullptr) {
+        return Status::ok();
+    }
+    sp<IBinder> binder = BnCarDataListener::asBinder(mCarDataListener);
+    auto status = binder->unlinkToDeath(mBinderDeathRecipient);
+    if (status != android::OK) {
+        LOG(WARNING) << "unlinkToDeath for CarDataListener failed, continuing anyway";
+    }
+    mCarDataListener = nullptr;
     return Status::ok();
 }
 
@@ -48,6 +83,18 @@ status_t CarTelemetryInternalImpl::dump(int fd, const android::Vector<android::S
     mRingBuffer->dump(fd);
     return android::OK;
 }
+
+// Removes the listener if its binder dies.
+void CarTelemetryInternalImpl::listenerBinderDied(const wp<android::IBinder>& what) {
+    const std::scoped_lock<std::mutex> lock(mMutex);
+    if (BnCarDataListener::asBinder(mCarDataListener) == what.unsafe_get()) {
+        LOG(WARNING) << "A CarDataListener died, removing the listener.";
+        mCarDataListener = nullptr;
+    } else {
+        LOG(ERROR) << "An unknown CarDataListener died, ignoring";
+    }
+}
+
 }  // namespace telemetry
 }  // namespace automotive
 }  // namespace android
diff --git a/cpp/telemetry/src/CarTelemetryInternalImpl.h b/cpp/telemetry/src/CarTelemetryInternalImpl.h
index 7b9fe73931..28c0a080e5 100644
--- a/cpp/telemetry/src/CarTelemetryInternalImpl.h
+++ b/cpp/telemetry/src/CarTelemetryInternalImpl.h
@@ -17,16 +17,17 @@
 #ifndef CPP_TELEMETRY_SRC_CARTELEMETRYINTERNALIMPL_H_
 #define CPP_TELEMETRY_SRC_CARTELEMETRYINTERNALIMPL_H_
 
+#include "RingBuffer.h"
+#include "binderutils/BinderDeathRecipient.h"
+
 #include <android/automotive/telemetry/internal/BnCarTelemetryInternal.h>
 #include <android/automotive/telemetry/internal/CarDataInternal.h>
+#include <android/automotive/telemetry/internal/ICarDataListener.h>
+#include <binder/IBinder.h>
+#include <utils/Mutex.h>
 #include <utils/String16.h>
 #include <utils/Vector.h>
 
-#include <RingBuffer.h>
-
-#include <memory>
-#include <vector>
-
 namespace android {
 namespace automotive {
 namespace telemetry {
@@ -47,7 +48,14 @@ public:
     status_t dump(int fd, const android::Vector<android::String16>& args) override;
 
 private:
+    void listenerBinderDied(const wp<android::IBinder>& what);
+
     RingBuffer* mRingBuffer;  // not owned
+    android::sp<BinderDeathRecipient> mBinderDeathRecipient;
+    std::mutex mMutex;  // a mutex for the whole instance
+
+    android::sp<android::automotive::telemetry::internal::ICarDataListener> mCarDataListener
+            GUARDED_BY(mMutex);
 };
 
 }  // namespace telemetry
diff --git a/cpp/telemetry/src/RingBuffer.h b/cpp/telemetry/src/RingBuffer.h
index db10a382a0..b9f8f9a03e 100644
--- a/cpp/telemetry/src/RingBuffer.h
+++ b/cpp/telemetry/src/RingBuffer.h
@@ -17,7 +17,7 @@
 #ifndef CPP_TELEMETRY_SRC_RINGBUFFER_H_
 #define CPP_TELEMETRY_SRC_RINGBUFFER_H_
 
-#include <BufferedCarData.h>
+#include "BufferedCarData.h"
 
 #include <list>
 
diff --git a/cpp/telemetry/src/binderutils/BinderDeathRecipient.cpp b/cpp/telemetry/src/binderutils/BinderDeathRecipient.cpp
new file mode 100644
index 0000000000..0d38efe1e9
--- /dev/null
+++ b/cpp/telemetry/src/binderutils/BinderDeathRecipient.cpp
@@ -0,0 +1,38 @@
+/*
+ * 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 "BinderDeathRecipient.h"
+
+#include <binder/IBinder.h>
+
+namespace android {
+namespace automotive {
+namespace telemetry {
+
+using ::android::IBinder;
+using ::android::wp;
+
+BinderDeathRecipient::BinderDeathRecipient(
+        std::function<void(const wp<android::IBinder>& what)> binderDiedCallback) :
+      mBinderDiedCallback(std::move(binderDiedCallback)) {}
+
+void BinderDeathRecipient::binderDied(const wp<IBinder>& binder) {
+    mBinderDiedCallback(binder);
+}
+
+}  // namespace telemetry
+}  // namespace automotive
+}  // namespace android
diff --git a/cpp/telemetry/src/binderutils/BinderDeathRecipient.h b/cpp/telemetry/src/binderutils/BinderDeathRecipient.h
new file mode 100644
index 0000000000..ebf9bb62b9
--- /dev/null
+++ b/cpp/telemetry/src/binderutils/BinderDeathRecipient.h
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+
+#ifndef CPP_TELEMETRY_SRC_BINDERUTILS_BINDERDEATHRECIPIENT_H_
+#define CPP_TELEMETRY_SRC_BINDERUTILS_BINDERDEATHRECIPIENT_H_
+
+#include <binder/IBinder.h>
+
+#include <functional>
+
+namespace android {
+namespace automotive {
+namespace telemetry {
+
+// Binder death recipient. See `android.os.IBinder#linkToDeath()` method to learn more.
+// Used to monitor status of the listeners/callbacks connected through binder.
+class BinderDeathRecipient : public android::IBinder::DeathRecipient {
+public:
+    explicit BinderDeathRecipient(
+            std::function<void(const wp<android::IBinder>& what)> binderDiedCallback);
+
+    void binderDied(const android::wp<android::IBinder>& binder) override;
+
+private:
+    std::function<void(const wp<android::IBinder>& what)> mBinderDiedCallback;
+};
+
+}  // namespace telemetry
+}  // namespace automotive
+}  // namespace android
+
+#endif  // CPP_TELEMETRY_SRC_BINDERUTILS_BINDERDEATHRECIPIENT_H_
diff --git a/cpp/telemetry/tests/CarTelemetryImplTest.cpp b/cpp/telemetry/tests/CarTelemetryImplTest.cpp
index f0af963ae8..af984617d0 100644
--- a/cpp/telemetry/tests/CarTelemetryImplTest.cpp
+++ b/cpp/telemetry/tests/CarTelemetryImplTest.cpp
@@ -29,11 +29,10 @@
 namespace android {
 namespace automotive {
 namespace telemetry {
-namespace {
 
-using android::frameworks::automotive::telemetry::CarData;
-using android::frameworks::automotive::telemetry::ICarTelemetry;
-using testing::ContainerEq;
+using ::android::frameworks::automotive::telemetry::CarData;
+using ::android::frameworks::automotive::telemetry::ICarTelemetry;
+using ::testing::ContainerEq;
 
 const size_t kMaxBufferSizeBytes = 1024;
 
@@ -93,7 +92,6 @@ TEST_F(CarTelemetryImplTest, TestWriteBuffersOnlyLimitedAmount) {
     EXPECT_EQ(buffer.currentSizeBytes(), 6);
 }
 
-}  // namespace
 }  // namespace telemetry
 }  // namespace automotive
 }  // namespace android
diff --git a/cpp/telemetry/tests/CarTelemetryInternalImplTest.cpp b/cpp/telemetry/tests/CarTelemetryInternalImplTest.cpp
new file mode 100644
index 0000000000..dc25422da6
--- /dev/null
+++ b/cpp/telemetry/tests/CarTelemetryInternalImplTest.cpp
@@ -0,0 +1,139 @@
+/*
+ * Copyright 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 "CarTelemetryInternalImpl.h"
+#include "RingBuffer.h"
+
+#include <android/automotive/telemetry/internal/CarDataInternal.h>
+#include <android/automotive/telemetry/internal/ICarDataListener.h>
+#include <android/automotive/telemetry/internal/ICarTelemetryInternal.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <unistd.h>
+
+#include <memory>
+
+namespace android {
+namespace automotive {
+namespace telemetry {
+
+using ::android::automotive::telemetry::internal::CarDataInternal;
+using ::android::automotive::telemetry::internal::ICarDataListener;
+using ::android::automotive::telemetry::internal::ICarTelemetryInternal;
+using ::android::binder::Status;
+
+const size_t kMaxBufferSizeBytes = 1024;
+
+class FakeBinder : public BBinder {
+public:
+    status_t linkToDeath(const sp<DeathRecipient>& recipient, void* cookie,
+                         uint32_t flags) override {
+        mDeathRecipient = recipient;
+        return mLinkToDeathStatus;
+    }
+
+    status_t unlinkToDeath(const wp<DeathRecipient>& recipient, void* cookie, uint32_t flags,
+                           wp<DeathRecipient>* outRecipient) override {
+        return android::OK;
+    }
+
+    sp<DeathRecipient> mDeathRecipient;
+    status_t mLinkToDeathStatus = android::OK;  // Result of linkToDeath() method.
+};
+
+// General flow for ICarDataListener is:
+//   1. Internal client calls ICarTelemetryInternal.setListener(client_listener)
+//   2. Binder constructs instance of ICarDataListener with its own Binder instance
+//       * in this test's case, these are FakeCarDataListener and FakeBinder
+class FakeCarDataListener : public ICarDataListener {
+public:
+    explicit FakeCarDataListener(android::sp<FakeBinder> binder) : mFakeBinder(binder) {}
+
+    IBinder* onAsBinder() override { return mFakeBinder.get(); }
+
+    android::binder::Status onCarDataReceived(
+            const std::vector<CarDataInternal>& dataList) override {
+        return Status::ok();
+    }
+
+private:
+    sp<FakeBinder> mFakeBinder;
+};
+
+// Main test class.
+class CarTelemetryInternalImplTest : public ::testing::Test {
+protected:
+    CarTelemetryInternalImplTest() :
+          mBuffer(RingBuffer(kMaxBufferSizeBytes)),
+          mTelemetryInternal(std::make_unique<CarTelemetryInternalImpl>(&mBuffer)),
+          mFakeCarDataListenerBinder(new FakeBinder()),
+          mFakeCarDataListener(new FakeCarDataListener(mFakeCarDataListenerBinder)) {}
+
+    RingBuffer mBuffer;
+    std::unique_ptr<ICarTelemetryInternal> mTelemetryInternal;
+    android::sp<FakeBinder> mFakeCarDataListenerBinder;  // For mFakeCarDataListener
+    android::sp<FakeCarDataListener> mFakeCarDataListener;
+};
+
+TEST_F(CarTelemetryInternalImplTest, TestSetListenerReturnsOk) {
+    auto status = mTelemetryInternal->setListener(mFakeCarDataListener);
+
+    EXPECT_TRUE(status.isOk()) << status;
+}
+
+TEST_F(CarTelemetryInternalImplTest, TestSetListenerFailsWhenAlreadySubscribed) {
+    mTelemetryInternal->setListener(mFakeCarDataListener);
+
+    auto status = mTelemetryInternal->setListener(new FakeCarDataListener(new FakeBinder()));
+
+    EXPECT_EQ(status.exceptionCode(), Status::EX_ILLEGAL_STATE);
+}
+
+TEST_F(CarTelemetryInternalImplTest, TestSetListenerFailsIfListenedIsDead) {
+    // The next call to linkToDeath() returns dead object, meaning the listener is not valid.
+    mFakeCarDataListenerBinder->mLinkToDeathStatus = android::DEAD_OBJECT;
+
+    auto status = mTelemetryInternal->setListener(mFakeCarDataListener);
+
+    EXPECT_EQ(status.exceptionCode(), Status::EX_ILLEGAL_STATE);
+}
+
+TEST_F(CarTelemetryInternalImplTest, TestClearListenerWorks) {
+    mTelemetryInternal->setListener(mFakeCarDataListener);
+
+    mTelemetryInternal->clearListener();
+    auto status = mTelemetryInternal->setListener(mFakeCarDataListener);
+
+    EXPECT_TRUE(status.isOk()) << status;
+}
+
+TEST_F(CarTelemetryInternalImplTest, TestListenerBinderDied) {
+    mTelemetryInternal->setListener(mFakeCarDataListener);  // old listener
+    EXPECT_NE(mFakeCarDataListenerBinder->mDeathRecipient, nullptr);
+
+    // the old listener died
+    mFakeCarDataListenerBinder->mDeathRecipient->binderDied(mFakeCarDataListenerBinder);
+
+    // new listener
+    auto status = mTelemetryInternal->setListener(new FakeCarDataListener(new FakeBinder()));
+
+    EXPECT_TRUE(status.isOk()) << status;
+}
+
+}  // namespace telemetry
+}  // namespace automotive
+}  // namespace android
diff --git a/cpp/telemetry/tests/RingBufferTest.cpp b/cpp/telemetry/tests/RingBufferTest.cpp
index 077705e695..134118d01a 100644
--- a/cpp/telemetry/tests/RingBufferTest.cpp
+++ b/cpp/telemetry/tests/RingBufferTest.cpp
@@ -26,7 +26,6 @@
 namespace android {
 namespace automotive {
 namespace telemetry {
-namespace {
 
 using testing::ContainerEq;
 
@@ -62,7 +61,6 @@ TEST(RingBufferTest, TestPopAllDataForIdRemovesFromBuffer) {
     EXPECT_EQ(buffer.currentSizeBytes(), 3);  // bytes, because only ID=103 left.
 }
 
-}  // namespace
 }  // namespace telemetry
 }  // namespace automotive
 }  // namespace android