diff options
author | 2025-02-24 22:37:02 +0000 | |
---|---|---|
committer | 2025-03-18 19:37:12 +0000 | |
commit | 727cbc09e054a64b639a878b60c7c288b4bb2d1e (patch) | |
tree | c4ae3c9813a30ed7e73a6ae5d7831da17883d0ff | |
parent | bcbec20c069ccb1f3d2faa34aec1857aec06bc8f (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.cpp | 45 |
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() { |