Avoid UI freezing when reading battery capacity/status
Reading battery capacity and status could fail and end up
with timeout after 5s for some input devices, for example:
8BitDo SN30 Pro+ gamepad.
Before reading those items, it would be better to release
`mLock` so that other threads could run.
Bug: 244088945
Test: Manually verified no UI freezing when calling
`inputDevice.batteryState.capacity` or
`inputDevice.batteryState.status`
Change-Id: Ibb6b8c999b7fd4fe7fbcc57264a1fe6ad74903dc
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp
index 06a38c8..2f594d6 100644
--- a/services/inputflinger/reader/EventHub.cpp
+++ b/services/inputflinger/reader/EventHub.cpp
@@ -1516,25 +1516,35 @@
}
std::optional<int32_t> EventHub::getBatteryCapacity(int32_t deviceId, int32_t batteryId) const {
- std::scoped_lock _l(mLock);
+ std::filesystem::path batteryPath;
+ {
+ // Do not read the sysfs node to get the battery state while holding
+ // the EventHub lock. For some peripheral devices, reading battery state
+ // can be broken and take 5+ seconds. Holding the lock in this case would
+ // block all other event processing during this time. For now, we assume this
+ // call never happens on the InputReader thread and read the sysfs node outside
+ // the lock to prevent event processing from being blocked by this call.
+ std::scoped_lock _l(mLock);
- const auto infos = getBatteryInfoLocked(deviceId);
- auto it = infos.find(batteryId);
- if (it == infos.end()) {
- return std::nullopt;
- }
+ const auto infos = getBatteryInfoLocked(deviceId);
+ auto it = infos.find(batteryId);
+ if (it == infos.end()) {
+ return std::nullopt;
+ }
+ batteryPath = it->second.path;
+ } // release lock
+
std::string buffer;
// Some devices report battery capacity as an integer through the "capacity" file
- if (base::ReadFileToString(it->second.path / BATTERY_NODES.at(InputBatteryClass::CAPACITY),
+ if (base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::CAPACITY),
&buffer)) {
return std::stoi(base::Trim(buffer));
}
// Other devices report capacity as an enum value POWER_SUPPLY_CAPACITY_LEVEL_XXX
// These values are taken from kernel source code include/linux/power_supply.h
- if (base::ReadFileToString(it->second.path /
- BATTERY_NODES.at(InputBatteryClass::CAPACITY_LEVEL),
+ if (base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::CAPACITY_LEVEL),
&buffer)) {
// Remove any white space such as trailing new line
const auto levelIt = BATTERY_LEVEL.find(base::Trim(buffer));
@@ -1547,15 +1557,27 @@
}
std::optional<int32_t> EventHub::getBatteryStatus(int32_t deviceId, int32_t batteryId) const {
- std::scoped_lock _l(mLock);
- const auto infos = getBatteryInfoLocked(deviceId);
- auto it = infos.find(batteryId);
- if (it == infos.end()) {
- return std::nullopt;
- }
+ std::filesystem::path batteryPath;
+ {
+ // Do not read the sysfs node to get the battery state while holding
+ // the EventHub lock. For some peripheral devices, reading battery state
+ // can be broken and take 5+ seconds. Holding the lock in this case would
+ // block all other event processing during this time. For now, we assume this
+ // call never happens on the InputReader thread and read the sysfs node outside
+ // the lock to prevent event processing from being blocked by this call.
+ std::scoped_lock _l(mLock);
+
+ const auto infos = getBatteryInfoLocked(deviceId);
+ auto it = infos.find(batteryId);
+ if (it == infos.end()) {
+ return std::nullopt;
+ }
+ batteryPath = it->second.path;
+ } // release lock
+
std::string buffer;
- if (!base::ReadFileToString(it->second.path / BATTERY_NODES.at(InputBatteryClass::STATUS),
+ if (!base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::STATUS),
&buffer)) {
ALOGE("Failed to read sysfs battery info: %s", strerror(errno));
return std::nullopt;