Clean up connection to sensor hidl service

getService() would block until hidl service up and running. Replace
retry delay with service registration listener to speed up.

Moreover, when getService() return nullptr, it means sensor hidl
service does not exist. Remove retry in this case and return failure.

Bug: 66916774
Test: test device boot
Test: adb shell "stop; start"
Test: kill sensor hidl process
Test: mod manifest file to remove sensor entry
Change-Id: I685971425e97e314699de4630d9adf400aba4af2

5 files changed, 157 insertions, 32 deletions

diff --git a/services/sensorservice/Android.bp b/services/sensorservice/Android.bp
index 8d381b1c31..a7f3a5235e 100644
--- a/services/sensorservice/Android.bp
+++ b/services/sensorservice/Android.bp
@@ -14,6 +14,7 @@ cc_library_shared {
         "RecentEventLogger.cpp",
         "RotationVectorSensor.cpp",
         "SensorDevice.cpp",
+        "SensorDeviceUtils.cpp",
         "SensorDirectConnection.cpp",
         "SensorEventConnection.cpp",
         "SensorFusion.cpp",
diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp
index da3b2758d1..fdb69d3ecc 100644
--- a/services/sensorservice/SensorDevice.cpp
+++ b/services/sensorservice/SensorDevice.cpp
@@ -26,11 +26,10 @@
 #include <cinttypes>
 #include <thread>
 
-using android::hardware::hidl_vec;
-
 using namespace android::hardware::sensors::V1_0;
 using namespace android::hardware::sensors::V1_0::implementation;
-
+using android::hardware::hidl_vec;
+using android::SensorDeviceUtils::HidlServiceRegistrationWaiter;
 
 namespace android {
 // ---------------------------------------------------------------------------
@@ -52,7 +51,8 @@ static status_t StatusFromResult(Result result) {
     }
 }
 
-SensorDevice::SensorDevice() : mHidlTransportErrors(20) {
+SensorDevice::SensorDevice()
+        : mHidlTransportErrors(20), mRestartWaiter(new HidlServiceRegistrationWaiter()) {
     if (!connectHidlService()) {
         return;
     }
@@ -87,35 +87,29 @@ SensorDevice::SensorDevice() : mHidlTransportErrors(20) {
 }
 
 bool SensorDevice::connectHidlService() {
-    // SensorDevice may wait upto 100ms * 10 = 1s for hidl service.
-    constexpr auto RETRY_DELAY = std::chrono::milliseconds(100);
+    // SensorDevice will wait for HAL service to start if HAL is declared in device manifest.
     size_t retry = 10;
 
-    while (true) {
-        int initStep = 0;
+    while (retry-- > 0) {
         mSensors = ISensors::getService();
-        if (mSensors != nullptr) {
-            ++initStep;
-            // Poke ISensor service. If it has lingering connection from previous generation of
-            // system server, it will kill itself. There is no intention to handle the poll result,
-            // which will be done since the size is 0.
-            if(mSensors->poll(0, [](auto, const auto &, const auto &) {}).isOk()) {
-                // ok to continue
-                break;
-            }
-            // hidl service is restarting, pointer is invalid.
-            mSensors = nullptr;
+        if (mSensors == nullptr) {
+            // no sensor hidl service found
+            break;
         }
 
-        if (--retry <= 0) {
-            ALOGE("Cannot connect to ISensors hidl service!");
+        mRestartWaiter->reset();
+        // Poke ISensor service. If it has lingering connection from previous generation of
+        // system server, it will kill itself. There is no intention to handle the poll result,
+        // which will be done since the size is 0.
+        if(mSensors->poll(0, [](auto, const auto &, const auto &) {}).isOk()) {
+            // ok to continue
             break;
         }
-        // Delay 100ms before retry, hidl service is expected to come up in short time after
-        // crash.
-        ALOGI("%s unsuccessful, try again soon (remaining retry %zu).",
-                (initStep == 0) ? "getService()" : "poll() check", retry);
-        std::this_thread::sleep_for(RETRY_DELAY);
+
+        // hidl service is restarting, pointer is invalid.
+        mSensors = nullptr;
+        ALOGI("%s unsuccessful, remaining retry %zu.", __FUNCTION__, retry);
+        mRestartWaiter->wait();
     }
     return (mSensors != nullptr);
 }
@@ -173,7 +167,7 @@ ssize_t SensorDevice::getSensorList(sensor_t const** list) {
 }
 
 status_t SensorDevice::initCheck() const {
-    return mSensors != NULL ? NO_ERROR : NO_INIT;
+    return mSensors != nullptr ? NO_ERROR : NO_INIT;
 }
 
 ssize_t SensorDevice::poll(sensors_event_t* buffer, size_t count) {
diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h
index fd6cee67ea..6d7505158d 100644
--- a/services/sensorservice/SensorDevice.h
+++ b/services/sensorservice/SensorDevice.h
@@ -17,6 +17,7 @@
 #ifndef ANDROID_SENSOR_DEVICE_H
 #define ANDROID_SENSOR_DEVICE_H
 
+#include "SensorDeviceUtils.h"
 #include "SensorServiceUtils.h"
 
 #include <sensor/Sensor.h>
@@ -39,12 +40,9 @@
 namespace android {
 
 // ---------------------------------------------------------------------------
-using SensorServiceUtil::Dumpable;
-using hardware::Return;
 
-class SensorDevice : public Singleton<SensorDevice>, public Dumpable {
+class SensorDevice : public Singleton<SensorDevice>, public SensorServiceUtil::Dumpable {
 public:
-
     class HidlTransportErrorLog {
      public:
 
@@ -105,7 +103,7 @@ public:
 private:
     friend class Singleton<SensorDevice>;
 
-    sp<android::hardware::sensors::V1_0::ISensors> mSensors;
+    sp<hardware::sensors::V1_0::ISensors> mSensors;
     Vector<sensor_t> mSensorList;
     std::unordered_map<int32_t, sensor_t*> mConnectedDynamicSensors;
 
@@ -174,6 +172,8 @@ private:
         }
         return std::move(ret);
     }
+    //TODO(b/67425500): remove waiter after bug is resolved.
+    sp<SensorDeviceUtils::HidlServiceRegistrationWaiter> mRestartWaiter;
 
     bool isClientDisabled(void* ident);
     bool isClientDisabledLocked(void* ident);
diff --git a/services/sensorservice/SensorDeviceUtils.cpp b/services/sensorservice/SensorDeviceUtils.cpp
new file mode 100644
index 0000000000..b1344becd7
--- /dev/null
+++ b/services/sensorservice/SensorDeviceUtils.cpp
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 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 "SensorDeviceUtils.h"
+
+#include <android/hardware/sensors/1.0/ISensors.h>
+#include <utils/Log.h>
+
+#include <chrono>
+#include <thread>
+
+using ::android::hardware::Void;
+using namespace android::hardware::sensors::V1_0;
+
+namespace android {
+namespace SensorDeviceUtils {
+
+HidlServiceRegistrationWaiter::HidlServiceRegistrationWaiter()
+        : mRegistered(ISensors::registerForNotifications("default", this)) {
+}
+
+Return<void> HidlServiceRegistrationWaiter::onRegistration(
+        const hidl_string &fqName, const hidl_string &name, bool preexisting) {
+    ALOGV("onRegistration fqName %s, name %s, preexisting %d",
+          fqName.c_str(), name.c_str(), preexisting);
+
+    {
+        std::lock_guard<std::mutex> lk(mLock);
+        mRestartObserved = true;
+    }
+    mCondition.notify_all();
+    return Void();
+}
+
+void HidlServiceRegistrationWaiter::reset() {
+    std::lock_guard<std::mutex> lk(mLock);
+    mRestartObserved = false;
+}
+
+bool HidlServiceRegistrationWaiter::wait() {
+    constexpr int DEFAULT_WAIT_MS = 100;
+    constexpr int TIMEOUT_MS = 1000;
+
+    if (!mRegistered) {
+        ALOGW("Cannot register service notification, use default wait(%d ms)", DEFAULT_WAIT_MS);
+        std::this_thread::sleep_for(std::chrono::milliseconds(DEFAULT_WAIT_MS));
+        // not sure if service is actually restarted
+        return false;
+    }
+
+    std::unique_lock<std::mutex> lk(mLock);
+    return mCondition.wait_for(lk, std::chrono::milliseconds(TIMEOUT_MS),
+            [this]{return mRestartObserved;});
+}
+
+} // namespace SensorDeviceUtils
+} // namespace android
diff --git a/services/sensorservice/SensorDeviceUtils.h b/services/sensorservice/SensorDeviceUtils.h
new file mode 100644
index 0000000000..da36928105
--- /dev/null
+++ b/services/sensorservice/SensorDeviceUtils.h
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2017 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 ANDROID_SENSOR_DEVICE_UTIL
+#define ANDROID_SENSOR_DEVICE_UTIL
+
+#include <android/hidl/manager/1.0/IServiceNotification.h>
+
+#include <condition_variable>
+#include <thread>
+
+using ::android::hardware::hidl_string;
+using ::android::hardware::Return;
+using ::android::hidl::manager::V1_0::IServiceNotification;
+
+namespace android {
+namespace SensorDeviceUtils {
+
+class HidlServiceRegistrationWaiter : public IServiceNotification {
+public:
+
+    HidlServiceRegistrationWaiter();
+
+    Return<void> onRegistration(const hidl_string &fqName,
+                                const hidl_string &name,
+                                bool preexisting) override;
+
+    void reset();
+
+    /**
+     * Wait for service restart
+     *
+     * @return true if service is restart since last reset(); false otherwise.
+     */
+    bool wait();
+private:
+    const bool mRegistered;
+
+    std::mutex mLock;
+    std::condition_variable mCondition;
+    bool mRestartObserved;
+};
+
+} // namespace SensorDeviceUtils
+} // namespace android;
+
+#endif // ANDROID_SENSOR_SERVICE_UTIL