Do not crash if InputClassifier HAL crashes
If InputClassifier HAL crashes for some reason, we don't have to crash
the system. Instead, just check return type and move on. The
InputClassifier stage is not critical to input dispatch, and the phone
can remain perfectly functional without this stage.
Log an error message instead.
We are also assuming in InputClassifier that HAL is always present.
There are 2 lines of defense here:
1) MotionClassifier always checks the returns from the HAL. If any of
the returns are not OK, then MotionClassifier thread exits. This is safe
to do always, but a downside of this is that logspam will occur if
events are not able to be added to the queue (since the thread that is
to be consuming them is no longer running).
2) Register HAL death recipient in InputClassifier. When the HAL death
occurs, mMotionClassifier will be set to null, thus
preventing further events from going into the queue. This will avoid the
logspam from 1).
Test: ps -A | grep -i input. Then interact with the phone. Then kill the
HAL process, 'killall
android.hardware.input.classifier@1.0-service-example'. Then make sure
that phone remains functional.
Bug: 117935272
Change-Id: I7e8f676d3baa0703198f0731273678c3575bdf60
diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp
index 3905ed0..b4db338 100644
--- a/services/inputflinger/InputClassifier.cpp
+++ b/services/inputflinger/InputClassifier.cpp
@@ -40,6 +40,7 @@
using android::base::StringPrintf;
using android::hardware::hidl_bitfield;
using android::hardware::hidl_vec;
+using android::hardware::Return;
using namespace android::hardware::input;
namespace android {
@@ -548,23 +549,28 @@
ensureHalThread(__func__);
while (true) {
ClassifierEvent event = mEvents.pop();
+ bool halResponseOk = true;
switch (event.type) {
case ClassifierEventType::MOTION: {
NotifyMotionArgs* motionArgs = static_cast<NotifyMotionArgs*>(event.args.get());
common::V1_0::MotionEvent motionEvent = getMotionEvent(*motionArgs);
- common::V1_0::Classification halClassification = mService->classify(motionEvent);
- updateClassification(motionArgs->deviceId, motionArgs->eventTime,
- getMotionClassification(halClassification));
+ Return<common::V1_0::Classification> response = mService->classify(motionEvent);
+ halResponseOk = response.isOk();
+ if (halResponseOk) {
+ common::V1_0::Classification halClassification = response;
+ updateClassification(motionArgs->deviceId, motionArgs->eventTime,
+ getMotionClassification(halClassification));
+ }
break;
}
case ClassifierEventType::DEVICE_RESET: {
const int32_t deviceId = *(event.getDeviceId());
- mService->resetDevice(deviceId);
+ halResponseOk = mService->resetDevice(deviceId).isOk();
setClassification(deviceId, MotionClassification::NONE);
break;
}
case ClassifierEventType::HAL_RESET: {
- mService->reset();
+ halResponseOk = mService->reset().isOk();
clearClassifications();
break;
}
@@ -573,6 +579,21 @@
return;
}
}
+ if (!halResponseOk) {
+ ALOGE("Error communicating with InputClassifier HAL. "
+ "Exiting MotionClassifier HAL thread");
+ clearClassifications();
+ return;
+ }
+ }
+}
+
+void MotionClassifier::enqueueEvent(ClassifierEvent&& event) {
+ bool eventAdded = mEvents.push(std::move(event));
+ if (!eventAdded) {
+ // If the queue is full, suspect the HAL is slow in processing the events.
+ ALOGE("Dropped event with eventTime %" PRId64, event.args->eventTime);
+ reset();
}
}
@@ -617,22 +638,12 @@
}
MotionClassification MotionClassifier::classify(const NotifyMotionArgs& args) {
- if (!mService) {
- // If HAL is not present, do nothing
- return MotionClassification::NONE;
- }
if ((args.action & AMOTION_EVENT_ACTION_MASK) == AMOTION_EVENT_ACTION_DOWN) {
updateLastDownTime(args.deviceId, args.downTime);
}
ClassifierEvent event(std::make_unique<NotifyMotionArgs>(args));
- bool elementAdded = mEvents.push(std::move(event));
- if (!elementAdded) {
- // Queue should not ever overfill. Suspect HAL is slow.
- ALOGE("Dropped element with eventTime %" PRIu64, args.eventTime);
- reset();
- return MotionClassification::NONE;
- }
+ enqueueEvent(std::move(event));
return getClassification(args.deviceId);
}
@@ -652,7 +663,7 @@
std::optional<int32_t> eventDeviceId = event.getDeviceId();
return eventDeviceId && (*eventDeviceId == deviceId);
});
- mEvents.push(std::make_unique<NotifyDeviceResetArgs>(args));
+ enqueueEvent(std::make_unique<NotifyDeviceResetArgs>(args));
}
void MotionClassifier::dump(std::string& dump) {
@@ -679,20 +690,39 @@
}
}
+
// --- InputClassifier ---
InputClassifier::InputClassifier(const sp<InputListenerInterface>& listener) :
mListener(listener) {
- if (deepPressEnabled()) {
- sp<android::hardware::input::classifier::V1_0::IInputClassifier> service =
- classifier::V1_0::IInputClassifier::getService();
- if (service) {
- mMotionClassifier = std::make_unique<MotionClassifier>(service);
- } else {
- ALOGI("Could not obtain InputClassifier HAL");
- }
+ // The rest of the initialization is done in onFirstRef, because we need to obtain
+ // an sp to 'this' in order to register for HAL death notifications
+}
+
+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");
+ return;
+ }
+
+ mMotionClassifier = std::make_unique<MotionClassifier>(service);
+}
void InputClassifier::notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) {
// pass through
@@ -705,12 +735,17 @@
}
void InputClassifier::notifyMotion(const NotifyMotionArgs* args) {
- NotifyMotionArgs copyArgs = NotifyMotionArgs(*args);
- if (mMotionClassifier && isTouchEvent(*args)) {
- // We only cover touch events, for now.
- copyArgs.classification = mMotionClassifier->classify(copyArgs);
+ std::scoped_lock lock(mLock);
+ // MotionClassifier is only used for touch events, for now
+ const bool sendToMotionClassifier = mMotionClassifier && isTouchEvent(*args);
+ if (!sendToMotionClassifier) {
+ mListener->notifyMotion(args);
+ return;
}
- mListener->notifyMotion(©Args);
+
+ NotifyMotionArgs newArgs(*args);
+ newArgs.classification = mMotionClassifier->classify(newArgs);
+ mListener->notifyMotion(&newArgs);
}
void InputClassifier::notifySwitch(const NotifySwitchArgs* args) {
@@ -719,6 +754,7 @@
}
void InputClassifier::notifyDeviceReset(const NotifyDeviceResetArgs* args) {
+ std::scoped_lock lock(mLock);
if (mMotionClassifier) {
mMotionClassifier->reset(*args);
}
@@ -726,7 +762,19 @@
mListener->notifyDeviceReset(args);
}
+void InputClassifier::serviceDied(uint64_t /*cookie*/,
+ const wp<android::hidl::base::V1_0::IBase>& who) {
+ std::scoped_lock lock(mLock);
+ ALOGE("InputClassifier HAL has died. Setting mMotionClassifier to null");
+ mMotionClassifier = nullptr;
+ sp<android::hidl::base::V1_0::IBase> service = who.promote();
+ if (service) {
+ service->unlinkToDeath(this);
+ }
+}
+
void InputClassifier::dump(std::string& dump) {
+ std::scoped_lock lock(mLock);
dump += "Input Classifier State:\n";
dump += INDENT1 "Motion Classifier:\n";
diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h
index b97357d..0b1483f 100644
--- a/services/inputflinger/InputClassifier.h
+++ b/services/inputflinger/InputClassifier.h
@@ -69,7 +69,13 @@
* provide a MotionClassification for the current gesture.
*/
virtual MotionClassification classify(const NotifyMotionArgs& args) = 0;
+ /**
+ * Reset all internal HAL state.
+ */
virtual void reset() = 0;
+ /**
+ * Reset HAL state for a specific device.
+ */
virtual void reset(const NotifyDeviceResetArgs& args) = 0;
/**
@@ -107,6 +113,9 @@
*/
class MotionClassifier final : public MotionClassifierInterface {
public:
+ /**
+ * The provided pointer to the service cannot be null.
+ */
MotionClassifier(sp<android::hardware::input::classifier::V1_0::IInputClassifier> service);
~MotionClassifier();
/**
@@ -128,6 +137,10 @@
// The events that need to be sent to the HAL.
BlockingQueue<ClassifierEvent> mEvents;
/**
+ * Add an event to the queue mEvents.
+ */
+ void enqueueEvent(ClassifierEvent&& event);
+ /**
* Thread that will communicate with InputClassifier HAL.
* This should be the only thread that communicates with InputClassifier HAL,
* because this thread is allowed to block on the HAL calls.
@@ -143,9 +156,9 @@
*/
void callInputClassifierHal();
/**
- * Access to the InputClassifier HAL
+ * Access to the InputClassifier HAL. Can always be safely dereferenced.
*/
- sp<android::hardware::input::classifier::V1_0::IInputClassifier> mService;
+ const sp<android::hardware::input::classifier::V1_0::IInputClassifier> mService;
std::mutex mLock;
/**
* Per-device input classifications. Should only be accessed using the
@@ -184,15 +197,19 @@
void requestExit();
};
+
/**
* Implementation of the InputClassifierInterface.
* Represents a separate stage of input processing. All of the input events go through this stage.
* Acts as a passthrough for all input events except for motion events.
* The events of motion type are sent to MotionClassifier.
*/
-class InputClassifier : public InputClassifierInterface {
+class InputClassifier : public InputClassifierInterface,
+ public android::hardware::hidl_death_recipient {
public:
explicit InputClassifier(const sp<InputListenerInterface>& listener);
+ // Some of the constructor logic is finished in onFirstRef
+ virtual void onFirstRef() override;
virtual void notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) override;
virtual void notifyKey(const NotifyKeyArgs* args) override;
@@ -200,10 +217,15 @@
virtual void notifySwitch(const NotifySwitchArgs* args) override;
virtual void notifyDeviceReset(const NotifyDeviceResetArgs* args) override;
+ virtual void serviceDied(uint64_t cookie,
+ const wp<android::hidl::base::V1_0::IBase>& who) override;
+
virtual void dump(std::string& dump) override;
private:
- std::unique_ptr<MotionClassifierInterface> mMotionClassifier = nullptr;
+ // Protect access to mMotionClassifier, since it may become null via a hidl callback
+ std::mutex mLock;
+ std::unique_ptr<MotionClassifierInterface> mMotionClassifier GUARDED_BY(mLock);
// The next stage to pass input events to
sp<InputListenerInterface> mListener;
};