Skip to content

Keep epoll interest alive while a duplicate of the registered fd survives - #1230

Open
Will Portnoy (willportnoy) wants to merge 1 commit into
mainfrom
wportnoy/epoll-dup-survival
Open

Keep epoll interest alive while a duplicate of the registered fd survives#1230
Will Portnoy (willportnoy) wants to merge 1 commit into
mainfrom
wportnoy/epoll-dup-survival

Conversation

@willportnoy

Copy link
Copy Markdown
Member

Problem

Linux epoll(7) removes a descriptor from an interest list only after every descriptor referring to the underlying open file description (OFD) has been closed. The shim instead anchored each interest to the per-descriptor TypedFd, so closing the registered descriptor dropped the interest even when a dup referring to the same OFD was still open — EpollEntry::poll bailed on a dead Weak<TypedFd> and readiness was never delivered.

Fix

Anchor epoll interest to the open file description:

  • Add WeakEntryHandle to litebox::fd — a durable, dup-surviving weak reference to a descriptor's shared entry, plus EntryHandle::downgrade/as_ptr/with_shared_metadata.
  • Re-point epoll's DescriptorRef at a per-subsystem WeakEntryHandle, key interests by the OFD's stable address, and re-poll through the shared entry (eventfd/unix/pipe via the entry's IOPollable, socket/file via aliased metadata). Observer registration is unchanged; it already targets the shared pollable.
  • Expose with_iopollable on the pipe entry so a pipe can be polled without a live per-descriptor PipeFd.

Covers all six epoll-able fd types on main (eventfd, unix, pipe, socket, file, and the pre-existing epoll-on-epoll unimplemented!()), via exhaustive matches.

Test

tests/epoll_dup.c: register an eventfd, dup it, close the original, and verify the interest still delivers events and survives re-arming. Passes natively, fails without this change under Litebox, and passes with it.

…red fd survives

Linux epoll(7) removes a file descriptor from an interest list only after
every descriptor referring to the underlying open file description has
been closed. The shim instead anchored each interest to the
per-descriptor `TypedFd`, so closing the registered descriptor dropped
the interest even when a `dup` referring to the same open file
description remained open: `EpollEntry::poll` bailed on a dead
`Weak<TypedFd>` and the readiness was never delivered.

Anchor epoll interest to the open file description instead:

- Add `WeakEntryHandle` to `litebox::fd`: a durable, dup-surviving weak
  reference to a descriptor's shared entry, plus `EntryHandle::downgrade`,
  `as_ptr`, and shared (open-file-description-level) metadata access.
- Re-point epoll's `DescriptorRef` at a per-subsystem `WeakEntryHandle`,
  key interests by the open file description's stable address, and re-poll
  through the shared entry (eventfd/unix/pipe via the entry's `IOPollable`,
  socket/file via aliased metadata). Observer registration is unchanged;
  it already targets the shared pollable.
- Expose `with_iopollable` on the pipe entry so a pipe can be polled
  without a live per-descriptor `PipeFd`.

Add tests/epoll_dup.c: register an eventfd, dup it, close the original,
and verify the interest still delivers events and survives re-arming. It
passes natively, fails without this change under Litebox, and passes with
it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1494e366-b3cf-4196-91a0-1430cb9d5cc8
@github-actions

Copy link
Copy Markdown

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

Comment thread litebox/src/fd/mod.rs
Comment on lines +614 to +618
pub fn upgrade(&self) -> Option<EntryHandle<Platform, Subsystem>> {
self.0
.upgrade()
.map(|entry| EntryHandle(entry, PhantomData))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One subtle issue with this lies on

pub(crate) fn close_and_duplicate_if_shared<
Subsystem: FdEnabledSubsystem,
F: FnOnce(&Subsystem::Entry) -> bool,
>(
&mut self,
fd: &TypedFd<Subsystem>,
can_close_immediately: F,
) -> Option<CloseResult<Subsystem>> {
let idx = fd.x.as_usize()?;
let Some(old) = self.entries[idx].take() else {
unreachable!();
};
if Arc::strong_count(&old.x) == 1 {
// Unique, so we can just return it if allowed.
if can_close_immediately(old.x.entry.read().as_subsystem::<Subsystem>()) {
fd.x.mark_as_closed();
let entry = Arc::into_inner(old.x)
.map(|shared| RwLock::into_inner(shared.entry))
.map(DescriptorEntry::into_subsystem_entry::<Subsystem>)
.unwrap();
Some(CloseResult::Closed(entry))

Now Arc::into_inner(old.x)...unwrap() may panic. Closing a socket cannot be done simply via Drop. It needs to take the ownership of the entry. One potential fix suggested by copilot is to replace it with Arc::try_unwrap and return CloseResult::Duplicated on the error path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants