Merge "Initialize MotionClassifier from a separate thread" into qt-dev
diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp
index b4db338..4a6efa6 100644
--- a/services/inputflinger/InputClassifier.cpp
+++ b/services/inputflinger/InputClassifier.cpp
@@ -507,19 +507,53 @@
// --- 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 @@
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::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 @@
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 @@
}
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
+ // 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;
}
-
- 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");
- 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 @@
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 0b1483f..47e20db 100644
--- a/services/inputflinger/InputClassifier.h
+++ b/services/inputflinger/InputClassifier.h
@@ -114,15 +114,22 @@
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 @@
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 @@
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 @@
*/
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 @@
* 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 1651057..7cc17a2 100644
--- a/services/inputflinger/tests/InputClassifier_test.cpp
+++ b/services/inputflinger/tests/InputClassifier_test.cpp
@@ -136,11 +136,7 @@
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 @@
// 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 @@
// 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 @@
// 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, 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