Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
14 changes: 9 additions & 5 deletions src/io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,11 @@ 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 holding none
/// itself (`FilePoll::deinit*` works on the raw slot pointer; `init_with_owner`
/// / `alloc_file_poll` never form one), so no two `&mut Store` coexist; the
/// `&mut self` of `on_update` further up the stack during an in-callback
/// deinit is the remaining exception, and that put only queues the slot.
Comment thread
robobun marked this conversation as resolved.
#[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 +1822,10 @@ 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. Passed as
// the pointer, not via `inner()`: the store may free the slot in there.
unsafe { FilePoll::deinit_force_unregister(self.0.as_ptr()) };
}
/// Single nonnull-asref accessor for the process-global uWS loop pointer.
///
Expand Down
62 changes: 39 additions & 23 deletions src/io/posix_event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,18 +385,48 @@ 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);
// put back via `Store::put`; Drop would be wrong here. The `deinit*` entry
// points take the slot pointer rather than `&mut self` because `Store::put`
// may free the slot before it returns, which a reference argument (protected
// until its call returns) does not allow.
Comment thread
robobun marked this conversation as resolved.

/// Returns the slot to the event loop's `Store`.
///
/// # Safety
/// `this` is a live slot from [`FilePoll::init`] on this thread and is not used afterwards.
Comment thread
robobun marked this conversation as resolved.
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`] that also unregisters a fired one-shot poll. Safety: as for `deinit`.
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`] with the context the poll was created on. Safety: as for `deinit`.
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) {
unsafe fn deinit_possibly_defer(this: *mut FilePoll, vm: EventLoopCtx, force_unregister: bool) {
// SAFETY: as for `deinit_with_vm`. The `&mut` the autoref forms ends
// with this statement, so this path holds no reference into the slot
// when the store takes it back.
let was_ever_registered = unsafe { (*this).clear_for_put(vm, force_unregister) };
// SAFETY: `this` is non-null per the contract above.
let slot = unsafe { ptr::NonNull::new_unchecked(this) };
vm.file_polls_mut().put(slot, vm, was_ever_registered);
}

/// Returns whether the poll was ever registered, which `Store::put` needs.
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 +436,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
56 changes: 30 additions & 26 deletions src/io/windows_event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,23 @@ 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());
// teardown returns the slot to the pool via `Store::put`. As on POSIX, the
// `deinit*` entry points take the slot pointer rather than `&mut self`
// because `Store::put` frees the slot before it returns.
Comment thread
robobun marked this conversation as resolved.

/// Returns the slot to the event loop's `Store`.
///
/// # Safety
/// `this` is a live slot from [`FilePoll::init`] on this thread and is not used afterwards.
Comment thread
robobun marked this conversation as resolved.
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`].
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 +114,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 +124,28 @@ impl FilePoll {
true
}

fn deinit_possibly_defer(&mut self, vm: EventLoopCtx, loop_: &mut WindowsLoop) {
/// Returns whether the poll was ever registered, which `Store::put` needs.
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);
}

pub(crate) fn deinit_with_vm(&mut self, 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()`).
was_ever_registered
}

/// Safety: as for [`FilePoll::deinit`]; `vm` is the context the poll was created on.
pub(crate) unsafe fn deinit_with_vm(this: *mut FilePoll, vm: EventLoopCtx) {
let loop_ = vm.loop_mut();
self.deinit_possibly_defer(vm, loop_);
// SAFETY: fn contract. The `&mut` the autoref forms ends with this
// statement, so this path holds no reference into the slot 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) };
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
28 changes: 22 additions & 6 deletions src/runtime/dns_jsc/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4782,26 +4782,40 @@ 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`.
///
/// Safety: `poll` is the live poll that fired (`__bun_run_file_poll`'s contract). It is
/// raw, as `FilePoll::deinit` takes it, since both the no-channel branch and (through
/// `on_dns_socket_state`) `Channel::process` may return it to the store.
Comment thread
robobun marked this conversation as resolved.
#[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 +4905,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