commit 06d46fda66f7e6cc144a195ac6dc065bca2e3616
author Yifan Hong <elsk@google.com> Wed Apr 19 18:45:30 2017 -0700
committer Yifan Hong <elsk@google.com> Wed Apr 19 19:00:45 2017 -0700
tree 1c6588ed6a76d908ddd080f1ae05ee1923accaef
parent 511544f212a9e675c10928b29551746374fb1637

Fix reference loop in IEventQueue implementation.

EventQueue's destructor is never called because
EventQueueLooperCallback has a strong reference to it,
and EventQueueLooperCallback is never destroyed because
the file descriptor is never removed from the looper.

Bug: 37280081
Test: camera works
Test: event queue is destroyed when client removes
its reference (from logcat)

Change-Id: I12347b6d1cca354288382555d2d9a20acfa59233

services/sensorservice/hidl/EventQueue.cpp

diff --git a/services/sensorservice/hidl/EventQueue.cpp b/services/sensorservice/hidl/EventQueue.cpp
index c0365e5..ff20066 100644
--- a/services/sensorservice/hidl/EventQueue.cpp
+++ b/services/sensorservice/hidl/EventQueue.cpp
@@ -27,7 +27,8 @@
 
 class EventQueueLooperCallback : public ::android::LooperCallback {
 public:
-    EventQueueLooperCallback(sp<EventQueue> queue, sp<IEventQueueCallback> callback)
+    EventQueueLooperCallback(sp<::android::SensorEventQueue> queue,
+                             sp<IEventQueueCallback> callback)
             : mQueue(queue), mCallback(callback) {
     }
 
@@ -35,7 +36,11 @@
 
         ASensorEvent event;
         ssize_t actual;
-        const sp<::android::SensorEventQueue>& internalQueue = mQueue->mInternalQueue;
+
+        auto internalQueue = mQueue.promote();
+        if (internalQueue == nullptr) {
+            return 1;
+        }
 
         while ((actual = internalQueue->read(&event, 1 /* count */)) > 0) {
             internalQueue->sendAck(&event, actual);
@@ -47,7 +52,7 @@
     }
 
 private:
-    sp<EventQueue> mQueue;
+    wp<::android::SensorEventQueue> mQueue;
     sp<IEventQueueCallback> mCallback;
 };
 
@@ -58,18 +63,18 @@
             : mLooper(looper),
               mInternalQueue(internalQueue) {
 
-    mLooper->addFd(mInternalQueue->getFd(), ALOOPER_POLL_CALLBACK, ALOOPER_EVENT_INPUT,
-            new EventQueueLooperCallback(this, callback), NULL /* data */);
+    mLooper->addFd(internalQueue->getFd(), ALOOPER_POLL_CALLBACK, ALOOPER_EVENT_INPUT,
+            new EventQueueLooperCallback(internalQueue, callback), NULL /* data */);
 }
 
-EventQueue::~EventQueue() {
+void EventQueue::onLastStrongRef(const void *id) {
+    IEventQueue::onLastStrongRef(id);
     mLooper->removeFd(mInternalQueue->getFd());
 }
 
 // Methods from ::android::frameworks::sensorservice::V1_0::IEventQueue follow.
 Return<Result> EventQueue::enableSensor(int32_t sensorHandle, int32_t samplingPeriodUs,
         int64_t maxBatchReportLatencyUs) {
-    // TODO implement
     return convertResult(mInternalQueue->enableSensor(sensorHandle, samplingPeriodUs,
             maxBatchReportLatencyUs, 0 /* reserved flags */));
 }

services/sensorservice/hidl/EventQueue.h

diff --git a/services/sensorservice/hidl/EventQueue.h b/services/sensorservice/hidl/EventQueue.h
index 87c614b..6be03b7 100644
--- a/services/sensorservice/hidl/EventQueue.h
+++ b/services/sensorservice/hidl/EventQueue.h
@@ -42,7 +42,7 @@
         sp<IEventQueueCallback> callback,
         sp<::android::Looper> looper,
         sp<::android::SensorEventQueue> internalQueue);
-    ~EventQueue();
+    void onLastStrongRef(const void *) override;
 
     // Methods from ::android::frameworks::sensorservice::V1_0::IEventQueue follow.
     Return<Result> enableSensor(int32_t sensorHandle, int32_t samplingPeriodUs, int64_t maxBatchReportLatencyUs) override;