Initialize MotionClassifier from a separate thread

We are currently calling getService directly from the thread that calls
InputManager constructor. That means, the function call directly affects
the start-up time of the system.
But InputClassifier HAL is not mission-critical, and the system can
function perfectly fine without it. Moreover, we are already allowing
for mMotionClassifier to be null in our code.

In this change, mMotionClassifier will become non-null right away, but
may not be initialized when it is first created. The initialization will
happen in a separate thread. Once initialized, MotionClassifier may
become null inside InputClassifier if the latter receives a HAL death
callback.

To account for the possibility of the MotionClassifier not being valid
right away, we are allowing mService variable in the MotionClassifier to
be nullable.

The contract for mService is as follows: mService is null by default. If
MotionClassifier initializes successfully (may take ~ 100 ms), then
mService will become non-null. Once non-null, it stays non-null.
This allows MotionClassifier's separate thread to not have to check the
nullness of mService.
Other threads, however, must assume that mService can be null. They are
checking the value with a lock held.

If MotionClassifier init fails, it will also call the deathRecipient's
serviceDied call, to signal that it is no longer useful. This should
prompt the owner of MotionClassifier to dispose it.

Bug: 130184032
Test: atest google/perf/boottime/boottime-test
Test: atest libinput_tests inputflinger_tests
Change-Id: Id4f9c316ef6725d96abebc55a96079946647eb39

3 files changed, 110 insertions, 54 deletions

diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp
index b4db338687..4a6efa67a7 100644
--- a/services/inputflinger/InputClassifier.cpp
+++ b/services/inputflinger/InputClassifier.cpp
@@ -507,19 +507,53 @@ std::optional<int32_t> ClassifierEvent::getDeviceId() const {
 
 // --- MotionClassifier ---
 
-MotionClassifier::MotionClassifier(
-        sp<android::hardware::input::classifier::V1_0::IInputClassifier> service) :
-        mEvents(MAX_EVENTS), mService(service) {
+MotionClassifier::MotionClassifier(sp<android::hardware::hidl_death_recipient> deathRecipient) :
+        mDeathRecipient(deathRecipient), mEvents(MAX_EVENTS) {
     mHalThread = std::thread(&MotionClassifier::callInputClassifierHal, this);
 #if defined(__linux__)
     // Set the thread name for debugging
     pthread_setname_np(mHalThread.native_handle(), "InputClassifier");
 #endif
+}
+
+/**
+ * This function may block for some time to initialize the HAL, so it should only be called
+ * from the "InputClassifier HAL" thread.
+ */
+bool MotionClassifier::init() {
+    ensureHalThread(__func__);
+    sp<android::hardware::input::classifier::V1_0::IInputClassifier> service =
+            classifier::V1_0::IInputClassifier::getService();
+    if (!service) {
+        // Not really an error, maybe the device does not have this HAL,
+        // but somehow the feature flag is flipped
+        ALOGI("Could not obtain InputClassifier HAL");
+        return false;
+    }
+
+    sp<android::hardware::hidl_death_recipient> recipient = mDeathRecipient.promote();
+    if (recipient != nullptr) {
+        const bool linked = service->linkToDeath(recipient, 0 /* cookie */).withDefault(false);
+        if (!linked) {
+            ALOGE("Could not link MotionClassifier to the HAL death");
+            return false;
+        }
+    }
+
     // Under normal operation, we do not need to reset the HAL here. But in the case where system
     // crashed, but HAL didn't, we may be connecting to an existing HAL process that might already
     // have received events in the past. That means, that HAL could be in an inconsistent state
     // once it receives events from the newly created MotionClassifier.
     mEvents.push(ClassifierEvent::createHalResetEvent());
+
+    {
+        std::scoped_lock lock(mLock);
+        if (mService) {
+            ALOGE("MotionClassifier::%s should only be called once", __func__);
+        }
+        mService = service;
+    }
+    return true;
 }
 
 MotionClassifier::~MotionClassifier() {
@@ -530,7 +564,7 @@ MotionClassifier::~MotionClassifier() {
 void MotionClassifier::ensureHalThread(const char* function) {
     if (DEBUG) {
         if (std::this_thread::get_id() != mHalThread.get_id()) {
-            ALOGE("Function %s should only be called from InputClassifier thread", function);
+            LOG_FATAL("Function %s should only be called from InputClassifier thread", function);
         }
     }
 }
@@ -547,6 +581,21 @@ void MotionClassifier::ensureHalThread(const char* function) {
  */
 void MotionClassifier::callInputClassifierHal() {
     ensureHalThread(__func__);
+    const bool initialized = init();
+    if (!initialized) {
+        // MotionClassifier no longer useful.
+        // Deliver death notification from a separate thread
+        // because ~MotionClassifier may be invoked, which calls mHalThread.join()
+        std::thread([deathRecipient = mDeathRecipient](){
+                sp<android::hardware::hidl_death_recipient> recipient = deathRecipient.promote();
+                if (recipient != nullptr) {
+                    recipient->serviceDied(0 /*cookie*/, nullptr);
+                }
+        }).detach();
+        return;
+    }
+    // From this point on, mService is guaranteed to be non-null.
+
     while (true) {
         ClassifierEvent event = mEvents.pop();
         bool halResponseOk = true;
@@ -666,10 +715,19 @@ void MotionClassifier::reset(const NotifyDeviceResetArgs& args) {
     enqueueEvent(std::make_unique<NotifyDeviceResetArgs>(args));
 }
 
+const char* MotionClassifier::getServiceStatus() REQUIRES(mLock) {
+    if (!mService) {
+        return "null";
+    }
+    if (mService->ping().isOk()) {
+        return "running";
+    }
+    return "not responding";
+}
+
 void MotionClassifier::dump(std::string& dump) {
     std::scoped_lock lock(mLock);
-    std::string serviceStatus = mService->ping().isOk() ? "running" : " not responding";
-    dump += StringPrintf(INDENT2 "mService status: %s\n", serviceStatus.c_str());
+    dump += StringPrintf(INDENT2 "mService status: %s\n", getServiceStatus());
     dump += StringPrintf(INDENT2 "mEvents: %zu element(s) (max=%zu)\n",
             mEvents.size(), MAX_EVENTS);
     dump += INDENT2 "mClassifications, mLastDownTimes:\n";
@@ -700,28 +758,14 @@ InputClassifier::InputClassifier(const sp<InputListenerInterface>& listener) :
 }
 
 void InputClassifier::onFirstRef() {
-    std::scoped_lock lock(mLock);
     if (!deepPressEnabled()) {
-        // If feature is not enabled, the InputClassifier will just be in passthrough
-        // mode, and will forward all events to the next InputListener, unmodified
-        return;
-    }
-
-    sp<android::hardware::input::classifier::V1_0::IInputClassifier> service =
-            classifier::V1_0::IInputClassifier::getService();
-    if (!service) {
-        // Not really an error, maybe the device does not have this HAL,
-        // but somehow the feature flag is flipped
-        ALOGI("Could not obtain InputClassifier HAL");
-        return;
-    }
-    const bool linked = service->linkToDeath(this, 0 /* cookie */).withDefault(false);
-    if (!linked) {
-        ALOGE("Could not link android::InputClassifier to the HAL death");
+        // If feature is not enabled, MotionClassifier should stay null to avoid unnecessary work.
+        // When MotionClassifier is null, InputClassifier will forward all events
+        // to the next InputListener, unmodified.
         return;
     }
-
-    mMotionClassifier = std::make_unique<MotionClassifier>(service);
+    std::scoped_lock lock(mLock);
+    mMotionClassifier = std::make_unique<MotionClassifier>(this);
 }
 
 void InputClassifier::notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) {
@@ -786,4 +830,4 @@ void InputClassifier::dump(std::string& dump) {
     dump += "\n";
 }
 
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h
index 0b1483fdd5..47e20dbf75 100644
--- a/services/inputflinger/InputClassifier.h
+++ b/services/inputflinger/InputClassifier.h
@@ -114,15 +114,22 @@ protected:
 class MotionClassifier final : public MotionClassifierInterface {
 public:
     /**
-     * The provided pointer to the service cannot be null.
+     * The deathRecipient will be subscribed to the HAL death. If the death recipient
+     * owns MotionClassifier and receives HAL death, it should delete its copy of it.
+     * The callback serviceDied will also be sent if the MotionClassifier itself fails
+     * to initialize. If the MotionClassifier fails to initialize, it is not useful, and
+     * should be deleted.
+     * If no death recipient is supplied, then the registration step will be skipped, so there will
+     * be no listeners registered for the HAL death. This is useful for testing
+     * MotionClassifier in isolation.
      */
-    MotionClassifier(sp<android::hardware::input::classifier::V1_0::IInputClassifier> service);
+    explicit MotionClassifier(sp<android::hardware::hidl_death_recipient> deathRecipient = nullptr);
     ~MotionClassifier();
+
     /**
      * Classifies events asynchronously; that is, it doesn't block events on a classification,
-     * but instead sends them over to the classifier HAL
-     * and after a classification is determined,
-     * it then marks the next event it sees in the stream with it.
+     * but instead sends them over to the classifier HAL and after a classification is
+     * determined, it then marks the next event it sees in the stream with it.
      *
      * Therefore, it is acceptable to have the classifications be delayed by 1-2 events
      * in a particular gesture.
@@ -134,6 +141,16 @@ public:
     virtual void dump(std::string& dump) override;
 
 private:
+    /**
+     * Initialize MotionClassifier.
+     * Return true if initializaion is successful.
+     */
+    bool init();
+    /**
+     * Entity that will be notified of the HAL death (most likely InputClassifier).
+     */
+    wp<android::hardware::hidl_death_recipient> mDeathRecipient;
+
     // The events that need to be sent to the HAL.
     BlockingQueue<ClassifierEvent> mEvents;
     /**
@@ -148,7 +165,7 @@ private:
     std::thread mHalThread;
     /**
      * Print an error message if the caller is not on the InputClassifier thread.
-     * Caller must supply the name of the calling function as __function__
+     * Caller must supply the name of the calling function as __func__
      */
     void ensureHalThread(const char* function);
     /**
@@ -156,9 +173,14 @@ private:
      */
     void callInputClassifierHal();
     /**
-     * Access to the InputClassifier HAL. Can always be safely dereferenced.
+     * Access to the InputClassifier HAL. May be null if init() hasn't completed yet.
+     * When init() successfully completes, mService is guaranteed to remain non-null and to not
+     * change its value until MotionClassifier is destroyed.
+     * This variable is *not* guarded by mLock in the InputClassifier thread, because
+     * that thread knows exactly when this variable is initialized.
+     * When accessed in any other thread, mService is checked for nullness with a lock.
      */
-    const sp<android::hardware::input::classifier::V1_0::IInputClassifier> mService;
+    sp<android::hardware::input::classifier::V1_0::IInputClassifier> mService;
     std::mutex mLock;
     /**
      * Per-device input classifications. Should only be accessed using the
@@ -195,6 +217,10 @@ private:
      * Useful for tests to ensure proper cleanup.
      */
     void requestExit();
+    /**
+     * Return string status of mService
+     */
+    const char* getServiceStatus() REQUIRES(mLock);
 };
 
 
diff --git a/services/inputflinger/tests/InputClassifier_test.cpp b/services/inputflinger/tests/InputClassifier_test.cpp
index 16510577bf..7cc17a2215 100644
--- a/services/inputflinger/tests/InputClassifier_test.cpp
+++ b/services/inputflinger/tests/InputClassifier_test.cpp
@@ -136,11 +136,7 @@ protected:
     std::unique_ptr<MotionClassifierInterface> mMotionClassifier;
 
     virtual void SetUp() override {
-        sp<android::hardware::input::classifier::V1_0::IInputClassifier> service =
-                classifier::V1_0::IInputClassifier::getService();
-        if (service) {
-            mMotionClassifier = std::make_unique<MotionClassifier>(service);
-        }
+        mMotionClassifier = std::make_unique<MotionClassifier>();
     }
 };
 
@@ -165,9 +161,7 @@ TEST_F(MotionClassifierTest, Classify_NoVideoFrames) {
 
     // We are not checking the return value, because we can't be making assumptions
     // about the HAL operation, since it will be highly hardware-dependent
-    if (mMotionClassifier) {
-        ASSERT_NO_FATAL_FAILURE(mMotionClassifier->classify(motionArgs));
-    }
+    ASSERT_NO_FATAL_FAILURE(mMotionClassifier->classify(motionArgs));
 }
 
 /**
@@ -183,9 +177,7 @@ TEST_F(MotionClassifierTest, Classify_OneVideoFrame) {
 
     // We are not checking the return value, because we can't be making assumptions
     // about the HAL operation, since it will be highly hardware-dependent
-    if (mMotionClassifier) {
-        ASSERT_NO_FATAL_FAILURE(mMotionClassifier->classify(motionArgs));
-    }
+    ASSERT_NO_FATAL_FAILURE(mMotionClassifier->classify(motionArgs));
 }
 
 /**
@@ -206,18 +198,14 @@ TEST_F(MotionClassifierTest, Classify_TwoVideoFrames) {
 
     // We are not checking the return value, because we can't be making assumptions
     // about the HAL operation, since it will be highly hardware-dependent
-    if (mMotionClassifier) {
-        ASSERT_NO_FATAL_FAILURE(mMotionClassifier->classify(motionArgs));
-    }
+    ASSERT_NO_FATAL_FAILURE(mMotionClassifier->classify(motionArgs));
 }
 
 /**
  * Make sure MotionClassifier does not crash when it is reset.
  */
 TEST_F(MotionClassifierTest, Reset_DoesNotCrash) {
-    if (mMotionClassifier) {
-        ASSERT_NO_FATAL_FAILURE(mMotionClassifier->reset());
-    }
+    ASSERT_NO_FATAL_FAILURE(mMotionClassifier->reset());
 }
 
 /**
@@ -225,9 +213,7 @@ TEST_F(MotionClassifierTest, Reset_DoesNotCrash) {
  */
 TEST_F(MotionClassifierTest, DeviceReset_DoesNotCrash) {
     NotifyDeviceResetArgs args(1/*sequenceNum*/, 2/*eventTime*/, 3/*deviceId*/);
-    if (mMotionClassifier) {
-        ASSERT_NO_FATAL_FAILURE(mMotionClassifier->reset(args));
-    }
+    ASSERT_NO_FATAL_FAILURE(mMotionClassifier->reset(args));
 }
 
 } // namespace android