Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,15 @@ impl EventLoopCtx {
/// Single backref-deref accessor for the per-thread `Store`. Same contract
/// as [`loop_mut`]: `pub(crate)`, `&self → &mut`, must NOT be called while
/// another `&mut Store` (or a `&mut FilePoll` that lives inside the inline
/// hive buffer) is live. Every in-crate caller is a leaf op that decays
/// any conflicting `&mut FilePoll` to a raw slot pointer first
/// (`deinit_possibly_defer`) or holds none (`init_with_owner`,
/// `alloc_file_poll`), so no two `&mut Store` ever coexist.
/// hive buffer) is live. Every in-crate caller is a leaf op that itself
/// holds no `&mut FilePoll` at the call: the `FilePoll::deinit*` path works
/// on the raw slot pointer and has dropped its statement-scoped reborrow by
/// then, and `init_with_owner` / `alloc_file_poll` never form one. So no
/// two `&mut Store` ever coexist. The one `&mut FilePoll` that can still be
/// live is the receiver of `on_update` when an owner deinits its poll from
/// inside the poll's own callback (the dispatch chain in
/// `posix_event_loop` is still `&mut self` based); that put only queues
/// the slot.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline]
fn file_polls_mut(&self) -> &'static mut Store {
// SAFETY: per-thread set-once pointer (`BackRef`-shaped); the event
Expand Down Expand Up @@ -1821,7 +1826,11 @@ impl FilePollRef {
}
#[inline]
pub(crate) fn deinit_force_unregister(self) {
self.inner().deinit_force_unregister();
// SAFETY: type invariant — `self.0` is the live slot `init` claimed, and
// every copy of this handle is dead once the owner calls this. Not
// routed through `inner()`: the store may free the slot inside the
// call, so it gets the pointer rather than a `&mut` (see `FilePoll::deinit`).
unsafe { FilePoll::deinit_force_unregister(self.0.as_ptr()) };
}
/// Single nonnull-asref accessor for the process-global uWS loop pointer.
///
Expand Down
84 changes: 62 additions & 22 deletions src/io/posix_event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,17 +386,71 @@ impl FilePoll {

// Note: not `impl Drop` — FilePoll is pool-allocated (HiveArray) and explicitly
// put back via `Store::put`; Drop would be wrong here.
pub fn deinit(&mut self) {
let ctx = get_vm_ctx(self.allocator_type);
self.deinit_possibly_defer(ctx, false);
//
// The `deinit*` entry points take the slot pointer the owner holds, not
// `&mut self`: `Store::put` may recycle the slot before it returns (a
// `Box` free once the hive has spilled to the heap) when the poll never
// went through `register*`, and a reference argument is protected, so must
// stay allocated, until the call it was passed to returns. (In-tree owners
// all register right after `init`, and `register*` marks the poll
// `WasEverRegistered` even when the syscall fails, so that branch is
// currently the store's contract rather than a path anything takes.)
Comment thread
robobun marked this conversation as resolved.
Outdated

/// Returns the slot to the event loop's `Store`.
///
/// # Safety
/// `this` is a live slot returned by [`FilePoll::init`] on this thread;
/// the caller must not use it afterwards.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub unsafe fn deinit(this: *mut FilePoll) {
// SAFETY: fn contract; the field read ends at the `;`.
let ctx = get_vm_ctx(unsafe { (*this).allocator_type });
// SAFETY: fn contract.
unsafe { Self::deinit_possibly_defer(this, ctx, false) }
}

/// [`FilePoll::deinit`], but also removes the kernel registration of a
/// fired one-shot poll, which `unregister` otherwise skips.
///
/// # Safety
/// As for [`FilePoll::deinit`].
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) unsafe fn deinit_force_unregister(this: *mut FilePoll) {
// SAFETY: fn contract; the field read ends at the `;`.
let ctx = get_vm_ctx(unsafe { (*this).allocator_type });
// SAFETY: fn contract.
unsafe { Self::deinit_possibly_defer(this, ctx, true) }
}

pub(crate) fn deinit_force_unregister(&mut self) {
let ctx = get_vm_ctx(self.allocator_type);
self.deinit_possibly_defer(ctx, true);
/// [`FilePoll::deinit`] for callers that already hold the poll's context.
///
/// # Safety
/// As for [`FilePoll::deinit`]; `vm` is the context the poll was created on.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub unsafe fn deinit_with_vm(this: *mut FilePoll, vm: EventLoopCtx) {
// SAFETY: fn contract.
unsafe { Self::deinit_possibly_defer(this, vm, false) }
}

fn deinit_possibly_defer(&mut self, vm: EventLoopCtx, force_unregister: bool) {
/// # Safety
/// As for [`FilePoll::deinit_with_vm`].
Comment thread
robobun marked this conversation as resolved.
Outdated
unsafe fn deinit_possibly_defer(this: *mut FilePoll, vm: EventLoopCtx, force_unregister: bool) {
// SAFETY: fn contract. The `&mut` the autoref forms ends with the
// statement, so this path holds no reference into the slot when the
// store takes it back. (A poll deinit'd from inside its own callback
// still has `on_update`'s `&mut self` live up the stack; a dispatched
// poll was registered, so that put is the deferred one and frees
// nothing under it.)
let was_ever_registered = unsafe { (*this).clear_for_put(vm, force_unregister) };
// SAFETY: `this` is non-null per fn contract.
let slot = unsafe { ptr::NonNull::new_unchecked(this) };
// `file_polls_mut()` is the per-thread set-once `Store` back-pointer
// (`BackRef`-shaped); `Store::put` touches `slot` only via raw-pointer
// ops (see its doc).
Comment thread
robobun marked this conversation as resolved.
Outdated
vm.file_polls_mut().put(slot, vm, was_ever_registered);
}

/// Unregisters and clears the poll; returns whether it was ever
/// registered, which is what decides whether `Store::put` recycles the
/// slot now or after the current event loop turn.
Comment thread
robobun marked this conversation as resolved.
Outdated
fn clear_for_put(&mut self, vm: EventLoopCtx, force_unregister: bool) -> bool {
// `loop_mut()` is the crate-private nonnull-asref accessor (single
// deref in `EventLoopCtx`); the `&mut Loop` is consumed by `unregister`
// and dropped before any `&mut Store` is materialised.
Expand All @@ -406,21 +460,7 @@ impl FilePoll {
let was_ever_registered = self.flags.contains(Flags::WasEverRegistered);
self.flags = FlagsSet::empty();
self.fd = INVALID_FD;
// `self` may live inside the `Store.hive` inline array, so a
// `&mut Store` taken while `&mut self` is live would assert unique
// access over the slot and invalidate `self`'s tag (Stacked Borrows).
// Decay `self` to a raw slot pointer first, *then* materialise the
// `&mut Store` via the crate-private backref-deref accessor.
let this = ptr::NonNull::from(self);
// `file_polls_mut()` is the per-thread set-once `Store` back-pointer
// (`BackRef`-shaped); `&mut self` has been retired to `this` above so
// the `&mut Store` it produces is the sole unique borrow into the hive.
// `Store::put` touches `this` only via raw-pointer ops (see its doc).
vm.file_polls_mut().put(this, vm, was_ever_registered);
}

pub fn deinit_with_vm(&mut self, vm: EventLoopCtx) {
self.deinit_possibly_defer(vm, false);
was_ever_registered
}

pub fn is_registered(&self) -> bool {
Expand Down
65 changes: 43 additions & 22 deletions src/io/windows_event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,29 @@ impl FilePoll {

// Note: not `impl Drop` — FilePoll lives in a HiveArray pool slot, not a Box;
// teardown returns the slot to the pool via `Store::put`.
pub fn deinit(&mut self) {
self.deinit_with_vm(js_vm_ctx());
//
// Like the POSIX `FilePoll`, the `deinit*` entry points take the slot
// pointer, not `&mut self`: for a poll that was never registered (on
// Windows, every poll; see `unregister`) `Store::put` recycles the slot
// before returning (a `Box` free once the hive has spilled to the heap),
// and a reference argument is protected, so must stay allocated, until the
// call it was passed to returns.
Comment thread
robobun marked this conversation as resolved.
Outdated

/// Returns the slot to the event loop's `Store`.
///
/// # Safety
/// `this` is a live slot returned by [`FilePoll::init`] on this thread;
/// the caller must not use it afterwards.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub unsafe fn deinit(this: *mut FilePoll) {
// SAFETY: fn contract.
unsafe { Self::deinit_with_vm(this, js_vm_ctx()) }
}

pub(crate) fn deinit_force_unregister(&mut self) {
self.deinit()
/// # Safety
/// As for [`FilePoll::deinit`].
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) unsafe fn deinit_force_unregister(this: *mut FilePoll) {
// SAFETY: fn contract.
unsafe { Self::deinit(this) }
}

pub(crate) fn unregister(&mut self, _loop: &mut WindowsLoop) -> bool {
Expand All @@ -104,7 +121,7 @@ impl FilePoll {
// ever sets the `Poll*` registration flags after construction (this
// module defines no `register`), and every in-tree constructor passes
// empty/default flags, so `is_registered()` stays false and
// `deinit_possibly_defer` — the only path here — never takes the
// `clear_for_put` — the only path here — never takes the
// `unregister` branch. If a Windows registration path is ever added,
// this cast must be replaced with a real `uv_handle_t` pointer first
// (see TODO above); `uv_unref` dereferences its argument.
Expand All @@ -114,34 +131,38 @@ impl FilePoll {
true
}

fn deinit_possibly_defer(&mut self, vm: EventLoopCtx, loop_: &mut WindowsLoop) {
/// Unregisters and clears the poll; returns whether it was ever
/// registered, which is what decides whether `Store::put` recycles the
/// slot now or after the current event loop turn.
Comment thread
robobun marked this conversation as resolved.
Outdated
fn clear_for_put(&mut self, loop_: &mut WindowsLoop) -> bool {
if self.is_registered() {
let _ = self.unregister(loop_);
}

let was_ever_registered = self.flags.contains(Flags::WasEverRegistered);
self.flags = FlagsSet::default();
self.fd = Fd::INVALID;
// All `self` field writes are done. Decay `self` to a raw slot pointer
// *before* materializing `&mut Store` so the `&mut Store` borrow (which
// covers the inline hive buffer) is the only live unique reference into
// that allocation when `Store::put` runs. `self` is never touched after
// this line — `Store::put` itself accesses `this` only via raw-pointer ops.
let this: ptr::NonNull<FilePoll> = ptr::NonNull::from(&mut *self);
// `file_polls_mut()` is the per-thread set-once `Store` back-pointer
// (`BackRef`-shaped); `&mut self` has been retired to `this` above so
// the `&mut Store` it produces is the sole unique borrow into the hive.
vm.file_polls_mut().put(this, vm, was_ever_registered);
was_ever_registered
}

pub(crate) fn deinit_with_vm(&mut self, vm: EventLoopCtx) {
/// # Safety
/// As for [`FilePoll::deinit`]; `vm` is the context the poll was created on.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) unsafe fn deinit_with_vm(this: *mut FilePoll, vm: EventLoopCtx) {
// `loop_mut()` — crate-private nonnull-asref accessor (single deref in
// `EventLoopCtx`); the uws loop is a disjoint allocation from `self`.
// Stacked-Borrows: `self` may live inside `Store.hive`'s inline buffer,
// so `&mut Store` is materialised only *after* `&mut self` is retired
// inside `deinit_possibly_defer` (via `file_polls_mut()`).
// `EventLoopCtx`); the uws loop is a disjoint allocation from the slot,
// and the borrow is consumed by `clear_for_put`.
Comment thread
robobun marked this conversation as resolved.
Outdated
let loop_ = vm.loop_mut();
self.deinit_possibly_defer(vm, loop_);
// SAFETY: fn contract. The `&mut` the autoref forms ends with the
// statement, so no reference into the slot is live when the store
// takes it back.
let was_ever_registered = unsafe { (*this).clear_for_put(loop_) };
// SAFETY: `this` is non-null per fn contract.
let slot = unsafe { ptr::NonNull::new_unchecked(this) };
// `file_polls_mut()` is the per-thread set-once `Store` back-pointer
// (`BackRef`-shaped); the `&mut Store` it produces is the only
// reference into the hive at this point. `Store::put` touches `slot`
// only via raw-pointer ops (see its doc).
Comment thread
robobun marked this conversation as resolved.
Outdated
vm.file_polls_mut().put(slot, vm, was_ever_registered);
}

pub(crate) fn enable_keeping_process_alive(&mut self, vm: EventLoopCtx) {
Expand Down
19 changes: 11 additions & 8 deletions src/runtime/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,10 +656,10 @@ pub(crate) fn tick_queue_with_count(
#[cfg(not(windows))]
#[unsafe(no_mangle)]
pub(crate) unsafe fn __bun_run_file_poll(poll: *mut FilePoll, size_or_offset: i64) {
// SAFETY: contract above.
let poll_ref = unsafe { &mut *poll };
let owner = poll_ref.owner;
let hup = poll_ref.flags.contains(PollFlag::Hup);
// SAFETY: contract above; both reads end at the `;`. The arms below get
// `poll` itself where they need it (the memory-pressure and DNS arms may
// return it to the store), so this frame keeps no reference into the slot.
let (owner, hup) = unsafe { ((*poll).owner, (*poll).flags.contains(PollFlag::Hup)) };

debug_assert!(!owner.is_null());

Expand Down Expand Up @@ -706,8 +706,9 @@ pub(crate) unsafe fn __bun_run_file_poll(poll: *mut FilePoll, size_or_offset: i6
unsafe { Process::on_wait_pid_from_event_loop_task(proc) };
}
poll_tag::MEMORY_PRESSURE => {
// SAFETY: `poll` is live per `__bun_run_file_poll`'s contract.
crate::node::memory_pressure::on_poll(unsafe { &mut *poll }, size_or_offset);
// SAFETY: `poll` is live per `__bun_run_file_poll`'s contract. Passed
// raw: `on_poll` may return the slot to the store.
unsafe { crate::node::memory_pressure::on_poll(poll, size_or_offset) };
}
poll_tag::PARENT_DEATH_WATCHDOG => {
let wd = owner_as!(bun_io::parent_death_watchdog::ParentDeathWatchdog);
Expand Down Expand Up @@ -740,8 +741,10 @@ pub(crate) unsafe fn __bun_run_file_poll(poll: *mut FilePoll, size_or_offset: i6
// `Channel::process` re-enters the resolver via c-ares callbacks.
// SAFETY: tag set with this pointee type at `FilePoll::init`.
let resolver = unsafe { &*owner.ptr.cast_const().cast::<DNSResolver>() };
// SAFETY: `poll` outlives this call (caller contract).
resolver.on_dns_poll(unsafe { &mut *poll });
// SAFETY: `poll` is live per `__bun_run_file_poll`'s contract and is
// the resolver's registered poll for its fd (it set this owner on
// it). Passed raw: `on_dns_poll` may return the slot to the store.
unsafe { resolver.on_dns_poll(poll) };
}
poll_tag::GET_ADDR_INFO_REQUEST => {
#[cfg(target_os = "macos")]
Expand Down
36 changes: 30 additions & 6 deletions src/runtime/dns_jsc/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4782,26 +4782,48 @@ impl Resolver {
/// UnsafeCell-backed, LLVM cannot cache `ref_count` across the FFI call —
/// the structural fix for the previously ASM-verified PROVEN_CACHED
/// miscompile that needed `black_box` laundering under `&mut self`.
///
/// `poll` is raw, the shape `FilePoll::deinit` takes: this either returns
/// the slot to the store itself (no channel) or runs `Channel::process`,
/// whose socket-state callback (`on_dns_socket_state`) returns it when
/// c-ares closes the socket, so the poll is read up front and not touched
/// afterwards. (A fired poll was registered, so either put only queues the
/// slot, and `on_update`'s `&mut self` is still live up the stack; this
/// just keeps this frame's parameter out of it.)
///
/// # Safety
/// `poll` is the live poll that fired, registered in `self.polls` for its fd
/// (`__bun_run_file_poll`'s contract).
Comment thread
robobun marked this conversation as resolved.
Outdated
#[cfg(not(windows))]
pub(crate) fn on_dns_poll(&self, poll: &mut FilePoll) {
pub(crate) unsafe fn on_dns_poll(&self, poll: *mut FilePoll) {
let vm = self.vm();
let _exit = vm.enter_event_loop_scope();
// SAFETY: fn contract; the read ends at the `;`.
let fd = unsafe { (*poll).fd.native() };
let Some(channel) = self.channel.get() else {
self.polls.with_mut(|p| {
let _ = p.remove(&poll.fd.native());
let _ = p.remove(&fd);
});
poll.deinit();
// SAFETY: fn contract; the map entry that held the slot is gone, so
// this is its last use.
unsafe { FilePoll::deinit(poll) };
return;
};

// SAFETY: `self` is the heap allocation from `init`; ref_scope keeps count > 0 across re-entrant callbacks.
let _deref = unsafe { Self::ref_scope(self.as_ctx_ptr()) };

// SAFETY: fn contract; each `&mut` the autoref forms ends with its
// statement, before `process` can reach the slot through the map.
let readable = unsafe { (*poll).is_readable() };
// SAFETY: as above.
let writable = unsafe { (*poll).is_writable() };

// SAFETY: `channel` is the live c-ares channel owned by `self`; no `&mut`
// to `*self` is held across this re-entrant call (all fields are
// UnsafeCell-backed).
unsafe {
(*channel).process(poll.fd.native(), poll.is_readable(), poll.is_writable());
(*channel).process(fd, readable, writable);
}

// c-ares detaches a query only *after* its callback returns, so
Expand Down Expand Up @@ -4891,8 +4913,10 @@ impl Resolver {
// the socket is now closed. We must free the data associated with
// socket.
if let Some(value) = self.polls.with_mut(|p| p.remove(&fd)) {
// SAFETY: `value` is the heap-allocated FilePoll for this fd.
unsafe { (*value).deinit_with_vm(ctx) };
// SAFETY: `value` is the live slot `FilePoll::init` returned
// for this fd below; removing it from the map was the only
// other reference to it.
unsafe { FilePoll::deinit_with_vm(value, ctx) };
}
return;
}
Expand Down
26 changes: 15 additions & 11 deletions src/runtime/dns_jsc/dns_sd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,18 +372,21 @@ impl SharedConnection {
(&raw mut *this).cast(),
),
);
// SAFETY: `FilePoll::init` returned a live pool slot; exclusive here.
let poll = unsafe { &mut *poll_ptr };
// SAFETY: the event loop outlives every lookup made on it.
let loop_ = unsafe { ctx.platform_event_loop() };
let rc = poll.register_with_fd(
loop_,
Async::PollKind::Readable,
Async::posix_event_loop::OneShotFlag::None,
fd,
);
// SAFETY: `FilePoll::init` returned a live pool slot that nothing else
// refers to yet; the `&mut` the autoref forms ends with the statement.
let rc = unsafe {
(*poll_ptr).register_with_fd(
loop_,
Async::PollKind::Readable,
Async::posix_event_loop::OneShotFlag::None,
fd,
)
};
if rc.is_err() {
poll.deinit();
// SAFETY: as above; nothing uses the slot after this.
unsafe { FilePoll::deinit(poll_ptr) };
// SAFETY: FFI; `main_ref` is the live connection ref.
unsafe { DNSServiceRefDeallocate(main_ref) };
return None;
Expand Down Expand Up @@ -598,8 +601,9 @@ impl SharedConnection {
.remove(conn.early_out_timer.as_ptr())
};
}
// SAFETY: `file_poll` is the live hive slot; `deinit` returns it.
unsafe { (*conn.file_poll.as_ptr()).deinit() };
// SAFETY: `file_poll` is the live hive slot this connection owns, and
// `conn` (its only holder) is dropped below; `deinit` returns it.
unsafe { FilePoll::deinit(conn.file_poll.as_ptr()) };
// SAFETY: FFI; releases the primary ref (and any remaining subordinates).
unsafe { DNSServiceRefDeallocate(conn.main_ref) };
drop(conn);
Expand Down
Loading