summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Prabir Pradhan <prabirmsp@google.com> 2023-12-06 22:38:21 +0000
committer Prabir Pradhan <prabirmsp@google.com> 2023-12-07 16:28:31 +0000
commit95019ac8d7b5217aa4b606fa5fe775f192c6baeb (patch)
tree1875b3a15c209550d1f961502083e38a5558ebe7
parente209cdde92e60b36652fb078749500f8379c4e6b (diff)
Introduce thread safety to InputDeviceMetricsCollector
There are several threads that interact with the metrics collector, such as the Reader thread, Dispatcher thread, and binder threads for dumping. However, thread safety was never added to it, since it started off only running on the Reader thread. There is a high likelihood that this is the root cause of crashes that we see when dumping the metrics collector. Add a lock to the metrics collector to guard its state. Bug: 315193876 Test: atest inputflinger_tests Change-Id: I6a53014fbc533cf15516e8b079ee5b715b4aa58f
-rw-r--r--services/inputflinger/InputDeviceMetricsCollector.cpp66
-rw-r--r--services/inputflinger/InputDeviceMetricsCollector.h27
-rw-r--r--services/inputflinger/InputManager.cpp3
3 files changed, 68 insertions, 28 deletions
diff --git a/services/inputflinger/InputDeviceMetricsCollector.cpp b/services/inputflinger/InputDeviceMetricsCollector.cpp
index cefb14059c..b5cb3cbb57 100644
--- a/services/inputflinger/InputDeviceMetricsCollector.cpp
+++ b/services/inputflinger/InputDeviceMetricsCollector.cpp
@@ -125,58 +125,84 @@ InputDeviceMetricsCollector::InputDeviceMetricsCollector(InputListenerInterface&
void InputDeviceMetricsCollector::notifyInputDevicesChanged(
const NotifyInputDevicesChangedArgs& args) {
- reportCompletedSessions();
- onInputDevicesChanged(args.inputDeviceInfos);
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ onInputDevicesChanged(args.inputDeviceInfos);
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifyConfigurationChanged(
const NotifyConfigurationChangedArgs& args) {
- reportCompletedSessions();
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifyKey(const NotifyKeyArgs& args) {
- reportCompletedSessions();
- const SourceProvider getSources = [&args](const MetricsDeviceInfo& info) {
- return std::set{getUsageSourceForKeyArgs(info.keyboardType, args)};
- };
- onInputDeviceUsage(DeviceId{args.deviceId}, nanoseconds(args.eventTime), getSources);
-
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ const SourceProvider getSources = [&args](const MetricsDeviceInfo& info) {
+ return std::set{getUsageSourceForKeyArgs(info.keyboardType, args)};
+ };
+ onInputDeviceUsage(DeviceId{args.deviceId}, nanoseconds(args.eventTime), getSources);
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifyMotion(const NotifyMotionArgs& args) {
- reportCompletedSessions();
- onInputDeviceUsage(DeviceId{args.deviceId}, nanoseconds(args.eventTime),
- [&args](const auto&) { return getUsageSourcesForMotionArgs(args); });
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ onInputDeviceUsage(DeviceId{args.deviceId}, nanoseconds(args.eventTime),
+ [&args](const auto&) { return getUsageSourcesForMotionArgs(args); });
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifySwitch(const NotifySwitchArgs& args) {
- reportCompletedSessions();
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifySensor(const NotifySensorArgs& args) {
- reportCompletedSessions();
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifyVibratorState(const NotifyVibratorStateArgs& args) {
- reportCompletedSessions();
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifyDeviceReset(const NotifyDeviceResetArgs& args) {
- reportCompletedSessions();
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ }
mNextListener.notify(args);
}
void InputDeviceMetricsCollector::notifyPointerCaptureChanged(
const NotifyPointerCaptureChangedArgs& args) {
- reportCompletedSessions();
+ {
+ std::scoped_lock lock(mLock);
+ reportCompletedSessions();
+ }
mNextListener.notify(args);
}
@@ -185,10 +211,12 @@ void InputDeviceMetricsCollector::notifyDeviceInteraction(int32_t deviceId, nsec
if (isIgnoredInputDeviceId(deviceId)) {
return;
}
+ std::scoped_lock lock(mLock);
mInteractionsQueue.push(DeviceId{deviceId}, timestamp, uids);
}
void InputDeviceMetricsCollector::dump(std::string& dump) {
+ std::scoped_lock lock(mLock);
dump += "InputDeviceMetricsCollector:\n";
dump += " Logged device IDs: " + dumpMapKeys(mLoggedDeviceInfos, &toString) + "\n";
@@ -196,6 +224,10 @@ void InputDeviceMetricsCollector::dump(std::string& dump) {
dumpMapKeys(mActiveUsageSessions, &toString) + "\n";
}
+void InputDeviceMetricsCollector::monitor() {
+ std::scoped_lock lock(mLock);
+}
+
void InputDeviceMetricsCollector::onInputDevicesChanged(const std::vector<InputDeviceInfo>& infos) {
std::map<DeviceId, MetricsDeviceInfo> newDeviceInfos;
diff --git a/services/inputflinger/InputDeviceMetricsCollector.h b/services/inputflinger/InputDeviceMetricsCollector.h
index 9633664a0e..1bcd527626 100644
--- a/services/inputflinger/InputDeviceMetricsCollector.h
+++ b/services/inputflinger/InputDeviceMetricsCollector.h
@@ -21,12 +21,14 @@
#include "NotifyArgs.h"
#include "SyncQueue.h"
+#include <android-base/thread_annotations.h>
#include <ftl/mixins.h>
#include <gui/WindowInfo.h>
#include <input/InputDevice.h>
#include <chrono>
#include <functional>
#include <map>
+#include <mutex>
#include <set>
#include <vector>
@@ -34,8 +36,6 @@ namespace android {
/**
* Logs metrics about registered input devices and their usages.
- *
- * All methods in the InputListenerInterface must be called from a single thread.
*/
class InputDeviceMetricsCollectorInterface : public InputListenerInterface {
public:
@@ -50,6 +50,9 @@ public:
* This method may be called on any thread (usually by the input manager on a binder thread).
*/
virtual void dump(std::string& dump) = 0;
+
+ /** Called by the heartbeat to ensure that this component has not deadlocked. */
+ virtual void monitor() = 0;
};
/** The logging interface for the metrics collector, injected for testing. */
@@ -116,10 +119,12 @@ public:
void notifyDeviceInteraction(int32_t deviceId, nsecs_t timestamp,
const std::set<gui::Uid>& uids) override;
void dump(std::string& dump) override;
+ void monitor() override;
private:
+ std::mutex mLock;
InputListenerInterface& mNextListener;
- InputDeviceMetricsLogger& mLogger;
+ InputDeviceMetricsLogger& mLogger GUARDED_BY(mLock);
const std::chrono::nanoseconds mUsageSessionTimeout;
// Type-safe wrapper for input device id.
@@ -135,10 +140,10 @@ private:
using Uid = gui::Uid;
using MetricsDeviceInfo = InputDeviceMetricsLogger::MetricsDeviceInfo;
- std::map<DeviceId, MetricsDeviceInfo> mLoggedDeviceInfos;
+ std::map<DeviceId, MetricsDeviceInfo> mLoggedDeviceInfos GUARDED_BY(mLock);
using Interaction = std::tuple<DeviceId, std::chrono::nanoseconds, std::set<Uid>>;
- SyncQueue<Interaction> mInteractionsQueue;
+ SyncQueue<Interaction> mInteractionsQueue GUARDED_BY(mLock);
class ActiveSession {
public:
@@ -166,16 +171,16 @@ private:
};
// The input devices that currently have active usage sessions.
- std::map<DeviceId, ActiveSession> mActiveUsageSessions;
+ std::map<DeviceId, ActiveSession> mActiveUsageSessions GUARDED_BY(mLock);
- void onInputDevicesChanged(const std::vector<InputDeviceInfo>& infos);
- void onInputDeviceRemoved(DeviceId deviceId, const MetricsDeviceInfo& info);
+ void onInputDevicesChanged(const std::vector<InputDeviceInfo>& infos) REQUIRES(mLock);
+ void onInputDeviceRemoved(DeviceId deviceId, const MetricsDeviceInfo& info) REQUIRES(mLock);
using SourceProvider =
std::function<std::set<InputDeviceUsageSource>(const MetricsDeviceInfo&)>;
void onInputDeviceUsage(DeviceId deviceId, std::chrono::nanoseconds eventTime,
- const SourceProvider& getSources);
- void onInputDeviceInteraction(const Interaction&);
- void reportCompletedSessions();
+ const SourceProvider& getSources) REQUIRES(mLock);
+ void onInputDeviceInteraction(const Interaction&) REQUIRES(mLock);
+ void reportCompletedSessions() REQUIRES(mLock);
};
} // namespace android
diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp
index 8cf61f9064..af4ba5a012 100644
--- a/services/inputflinger/InputManager.cpp
+++ b/services/inputflinger/InputManager.cpp
@@ -228,6 +228,9 @@ void InputManager::monitor() {
mReader->monitor();
mBlocker->monitor();
mProcessor->monitor();
+ if (ENABLE_INPUT_DEVICE_USAGE_METRICS) {
+ mCollector->monitor();
+ }
mDispatcher->monitor();
}