Keep epoll interest alive while a duplicate of the registered fd survives by willportnoy · Pull Request #1230 · microsoft/litebox

Will Portnoy (willportnoy) · 2026-08-27T14:50:22Z

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

…criptor `close_and_duplicate_if_shared` checked `Arc::strong_count == 1` and then took ownership with `Arc::into_inner(...).unwrap()`. With durable `WeakEntryHandle`s now held by epoll interests, another thread can `upgrade()` the shared entry lock-free (bypassing the descriptor-table lock) between the count check and the unwrap, transiently raising the strong count and panicking the unwrap. Take ownership with `Arc::try_unwrap` instead and, on the error path, put the entry back and return `CloseResult::Duplicated` so the descriptor is closed once the transient reference drops — the same self-healing path already used when the entry is genuinely shared. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1494e366-b3cf-4196-91a0-1430cb9d5cc8

…rvival # Conflicts: # litebox_shim_linux/src/syscalls/epoll.rs

github-actions · 2026-08-29T22:20:02Z

🤖 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.

Jay Bosamiya (Microsoft) (jaybosamiya-ms)

Thanks Will, I reviewed the changes in the litebox core, since this is changing our API surface. I think some changes are needed to make this more compatible with future evolution. The base of the changes is mostly reasonable, just needs some tweaks.

The changes should be fairly straightforward, but do let me know if you'd prefer I open up a PR with the changes I have in mind to the core, instead of tweaking those things over this PR.

Oh also, please do make sure to keep the PR description human-written, not AI-generated-human-reviewed.

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T21:51:53Z

+    /// The address of the shared open file description this handle refers to.
+    ///
+    /// Duplicates of a descriptor share one open file description, so this
+    /// address is stable across `dup` and uniquely identifies the description
+    /// for as long as any duplicate keeps it alive.
+    #[must_use]
+    pub fn as_ptr(&self) -> *const () {
+        Arc::as_ptr(&self.0).cast()
+    }


I am not sure I agree with exposing a *const () here, over-broadens our interface and guarantees (if we introduce this, now we are promising that we will always give out the address specifically). The correct move is probably to expose an opaque PartialEq+Eq+Hash+Clone+Copy EntryStableKey. That would allow more stable utilization.

Also another issue with it being a *const () is that, it would prevent us from making the stronger guarantee in the future that re-allocations even at the same address would be caught. If we make it an opaque EntryStableKey we can strengthen things without breaking any API contract, and automatically things get better.

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T22:08:50Z

+    /// Runs `f` with the aliased (open-file-description-level) metadata of type
+    /// `T`, if present.
+    ///
+    /// This reads the metadata stored via [`Descriptors::set_entry_metadata`],
+    /// which is shared by every descriptor referring to the same open file
+    /// description. Returns `None` if no such metadata exists.
+    pub fn with_shared_metadata<T, R>(&self, f: impl FnOnce(&T) -> R) -> Option<R>
+    where
+        T: core::any::Any + Clone + Send + Sync,
+    {
+        self.0.entry.read().metadata.get::<T>().map(f)
+    }


Nit: doc comment + function naming could be better for consistency:

Suggested change

/// Runs `f` with the aliased (open-file-description-level) metadata of type

/// `T`, if present.

///

/// This reads the metadata stored via [`Descriptors::set_entry_metadata`],

/// which is shared by every descriptor referring to the same open file

/// description. Returns `None` if no such metadata exists.

pub fn with_shared_metadata<T, R>(&self, f: impl FnOnce(&T) -> R) -> Option<R>

where

T: core::any::Any + Clone + Send + Sync,

{

self.0.entry.read().metadata.get::<T>().map(f)

}

/// Apply `f` on metadata at the entry, if it exists.

///

/// In contrast to [`Descriptors::with_metadata`], this obtains entry-level metadata.

/// For FD-specific metadata, one necessarily needs the specific FD.

pub fn with_entry_metadata<T, R>(&self, f: impl FnOnce(&T) -> R) -> Option<R>

where

T: core::any::Any + Clone + Send + Sync,

{

self.0.entry.read().metadata.get::<T>().map(f)

}

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T22:17:36Z

+    /// Downgrades to a [`WeakEntryHandle`] that survives `dup`.
+    ///
+    /// The resulting handle upgrades for as long as *any* descriptor referring
+    /// to the same open file description remains open, even after the specific
+    /// descriptor this handle was obtained from has been closed.
+    #[must_use]
+    pub fn downgrade(&self) -> WeakEntryHandle<Platform, Subsystem> {
+        WeakEntryHandle(Arc::downgrade(&self.0), PhantomData)
+    }


Nit: doc comment, it is not about surviving dup, it is about getting a non-owning WeakEntryHandle, that's all. Also, we do not need to document upgrade here.

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T22:20:37Z

+/// A durable, `dup`-surviving weak reference to a descriptor's open file
+/// description.
+///
+/// Unlike a [`TypedFd`], which is tied to one descriptor slot, this upgrades as
+/// long as *any* descriptor referring to the same open file description is
+/// open. It is the correct anchor for interest that must outlive the closure of
+/// the specific descriptor it was registered against (for example, epoll
+/// interest, per Linux `epoll(7)` semantics).


Similarly, this is not dup-surviving, it is simply a weak reference to a descriptor entry. This is too much superfluous documentation. The upgrade stuff also is literally the obvious thing for any weak pointer, there is no reason to state that it works as long as something strong still exists. The whole doc comment here could just be:

Suggested change

/// A durable, `dup`-surviving weak reference to a descriptor's open file

/// description.

///

/// Unlike a [`TypedFd`], which is tied to one descriptor slot, this upgrades as

/// long as *any* descriptor referring to the same open file description is

/// open. It is the correct anchor for interest that must outlive the closure of

/// the specific descriptor it was registered against (for example, epoll

/// interest, per Linux `epoll(7)` semantics).

/// A weak-reference to a descriptor entry.

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T22:20:50Z

+/// interest, per Linux `epoll(7)` semantics).
+pub struct WeakEntryHandle<Platform: RawSyncPrimitivesProvider, Subsystem: FdEnabledSubsystem>(
+    Weak<SharedEntry<Platform>>,
+    PhantomData<fn(Subsystem) -> Subsystem>,


Why is this inconsistent with EntryHandle's PhantomData?

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T22:22:17Z

+    /// The address of the shared open file description, stable across `dup`.
+    ///
+    /// This is safe to use as a durable identity key. A [`WeakEntryHandle`]
+    /// keeps the underlying allocation reserved even after the open file
+    /// description is closed (every strong reference dropped), so this address
+    /// is never recycled for a different open file description while this handle
+    /// exists. The pointer is only ever compared, never dereferenced.
+    #[must_use]
+    pub fn as_ptr(&self) -> *const () {
+        self.0.as_ptr().cast()
+    }


Similarly, this should switch to EntryStableKey

Jay Bosamiya (Microsoft) (jaybosamiya-ms) · 2026-08-31T22:23:04Z

+    ///
+    /// This lets a holder of a durable entry handle poll the pipe without a
+    /// live per-descriptor [`PipeFd`], which is required so that an epoll
+    /// interest survives closing the registered descriptor while a duplicate
+    /// referring to the same open file description remains open.


This explanation is unnecessary to keep here

Weiteng Chen (CvvT) reviewed Aug 27, 2026

View reviewed changes

Comment thread litebox/src/fd/mod.rs

Will Portnoy (willportnoy) and others added 2 commits August 28, 2026 05:05

Merge remote-tracking branch 'origin/main' into wportnoy/epoll-dup-su…

89b6e3c

…rvival # Conflicts: # litebox_shim_linux/src/syscalls/epoll.rs

Jay Bosamiya (Microsoft) (jaybosamiya-ms) requested changes Aug 31, 2026

View reviewed changes

Navigation Menu

Uh oh!

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

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

Will Portnoy (willportnoy) commented Aug 27, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 29, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) left a comment •

edited

Loading

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Will Portnoy (willportnoy) commented Aug 27, 2026

Problem

Fix

Test

Uh oh!

Uh oh!

github-actions Bot commented Aug 29, 2026

Uh oh!

Jay Bosamiya (Microsoft) (jaybosamiya-ms) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Jay Bosamiya (Microsoft) (jaybosamiya-ms) left a comment •

edited

Loading