summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Prabir Pradhan <prabirmsp@google.com> 2025-02-24 22:37:02 +0000
committer Prabir Pradhan <prabirmsp@google.com> 2025-03-18 19:37:12 +0000
commit727cbc09e054a64b639a878b60c7c288b4bb2d1e (patch)
treec4ae3c9813a30ed7e73a6ae5d7831da17883d0ff
parentbcbec20c069ccb1f3d2faa34aec1857aec06bc8f (diff)
EventHub: Close opening device before deleting on sysfs change
We must unregister all fds from epoll before closing them to avoid the fds from remaining open and thus leaking. Prevent fd leaks by closing any opening device from epoll before closing the Device via its destructor when the sysfs node changes and triggers a device reopening. In this CL, we modify the closeDeviceLocked function to also look at mOpeningDevices when a device is being closed. Context: https://idea.popcount.org/2017-03-20-epoll-is-fundamentally-broken-22/ Bug: 397208968 Test: Presubmit Flag: EXEMPT bug fix Change-Id: I3de8a9413e68a0b7409e5d0bf56b9d76b7164b49
-rw-r--r--services/inputflinger/reader/EventHub.cpp45
1 files changed, 26 insertions, 19 deletions
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp
index 2fcb5d831f..08d51c32b5 100644
--- a/services/inputflinger/reader/EventHub.cpp
+++ b/services/inputflinger/reader/EventHub.cpp
@@ -2694,32 +2694,25 @@ void EventHub::sysfsNodeChanged(const std::string& sysfsNodePath) {
return changed;
};
- std::set<Device*> devicesToClose;
- std::set<std::string /*path*/> devicesToOpen;
+ std::set<Device*> devicesToReopen;
- // Check in opening devices. If its associated device changed,
- // the device should be removed from mOpeningDevices and needs to be opened again.
- std::erase_if(mOpeningDevices, [&](const auto& dev) {
+ // Check in opening devices.
+ for (const auto& dev : mOpeningDevices) {
if (isAssociatedDeviceChanged(*dev)) {
- devicesToOpen.emplace(dev->path);
- return true;
+ devicesToReopen.emplace(dev.get());
}
- return false;
- });
+ }
- // Check in already added device. If its associated device changed,
- // the device needs to be re-opened.
+ // Check in already added devices.
for (const auto& [id, dev] : mDevices) {
if (isAssociatedDeviceChanged(*dev)) {
- devicesToOpen.emplace(dev->path);
- devicesToClose.emplace(dev.get());
+ devicesToReopen.emplace(dev.get());
}
}
- for (auto* device : devicesToClose) {
- closeDeviceLocked(*device);
- }
- for (const auto& path : devicesToOpen) {
+ for (auto* device : devicesToReopen) {
+ const auto path = device->path;
+ closeDeviceLocked(*device); // The Device object is deleted by this function.
openDeviceLocked(path);
}
}
@@ -2817,9 +2810,23 @@ void EventHub::closeDeviceLocked(Device& device) {
releaseControllerNumberLocked(device.controllerNumber);
device.controllerNumber = 0;
device.close();
- mClosingDevices.push_back(std::move(mDevices[device.id]));
- mDevices.erase(device.id);
+ // Try to remove this device from mDevices.
+ if (auto it = mDevices.find(device.id); it != mDevices.end()) {
+ mClosingDevices.push_back(std::move(mDevices[device.id]));
+ mDevices.erase(device.id);
+ return;
+ }
+
+ // Try to remove this device from mOpeningDevices.
+ if (auto it = std::find_if(mOpeningDevices.begin(), mOpeningDevices.end(),
+ [&device](auto& d) { return d->id == device.id; });
+ it != mOpeningDevices.end()) {
+ mOpeningDevices.erase(it);
+ return;
+ }
+
+ LOG_ALWAYS_FATAL("%s: Device with id %d was not found!", __func__, device.id);
}
base::Result<void> EventHub::readNotifyLocked() {