Skip to content

Commit 727cbc0

Browse files
committed
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
1 parent bcbec20 commit 727cbc0

1 file changed

Lines changed: 26 additions & 19 deletions

File tree

services/inputflinger/reader/EventHub.cpp

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,32 +2694,25 @@ void EventHub::sysfsNodeChanged(const std::string& sysfsNodePath) {
26942694
return changed;
26952695
};
26962696

2697-
std::set<Device*> devicesToClose;
2698-
std::set<std::string /*path*/> devicesToOpen;
2697+
std::set<Device*> devicesToReopen;
26992698

2700-
// Check in opening devices. If its associated device changed,
2701-
// the device should be removed from mOpeningDevices and needs to be opened again.
2702-
std::erase_if(mOpeningDevices, [&](const auto& dev) {
2699+
// Check in opening devices.
2700+
for (const auto& dev : mOpeningDevices) {
27032701
if (isAssociatedDeviceChanged(*dev)) {
2704-
devicesToOpen.emplace(dev->path);
2705-
return true;
2702+
devicesToReopen.emplace(dev.get());
27062703
}
2707-
return false;
2708-
});
2704+
}
27092705

2710-
// Check in already added device. If its associated device changed,
2711-
// the device needs to be re-opened.
2706+
// Check in already added devices.
27122707
for (const auto& [id, dev] : mDevices) {
27132708
if (isAssociatedDeviceChanged(*dev)) {
2714-
devicesToOpen.emplace(dev->path);
2715-
devicesToClose.emplace(dev.get());
2709+
devicesToReopen.emplace(dev.get());
27162710
}
27172711
}
27182712

2719-
for (auto* device : devicesToClose) {
2720-
closeDeviceLocked(*device);
2721-
}
2722-
for (const auto& path : devicesToOpen) {
2713+
for (auto* device : devicesToReopen) {
2714+
const auto path = device->path;
2715+
closeDeviceLocked(*device); // The Device object is deleted by this function.
27232716
openDeviceLocked(path);
27242717
}
27252718
}
@@ -2817,9 +2810,23 @@ void EventHub::closeDeviceLocked(Device& device) {
28172810
releaseControllerNumberLocked(device.controllerNumber);
28182811
device.controllerNumber = 0;
28192812
device.close();
2820-
mClosingDevices.push_back(std::move(mDevices[device.id]));
28212813

2822-
mDevices.erase(device.id);
2814+
// Try to remove this device from mDevices.
2815+
if (auto it = mDevices.find(device.id); it != mDevices.end()) {
2816+
mClosingDevices.push_back(std::move(mDevices[device.id]));
2817+
mDevices.erase(device.id);
2818+
return;
2819+
}
2820+
2821+
// Try to remove this device from mOpeningDevices.
2822+
if (auto it = std::find_if(mOpeningDevices.begin(), mOpeningDevices.end(),
2823+
[&device](auto& d) { return d->id == device.id; });
2824+
it != mOpeningDevices.end()) {
2825+
mOpeningDevices.erase(it);
2826+
return;
2827+
}
2828+
2829+
LOG_ALWAYS_FATAL("%s: Device with id %d was not found!", __func__, device.id);
28232830
}
28242831

28252832
base::Result<void> EventHub::readNotifyLocked() {

0 commit comments

Comments
 (0)