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
1 change: 1 addition & 0 deletions .github/workflows/miri.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ on:
- "src/ptr/**"
- "src/resolve_builtins/**"
- "src/shell_parser/**"
- "src/threading/**"
- "src/wyhash/**"
- "scripts/rust-miri.ts"
- "Cargo.toml"
Expand Down
1 change: 1 addition & 0 deletions scripts/rust-miri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const MIRI_CRATES = [
"bun_ptr",
"bun_resolve_builtins",
"bun_shell_parser",
"bun_threading",
"bun_wyhash",
];

Expand Down
3 changes: 2 additions & 1 deletion src/bundler/ParseTask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2865,7 +2865,8 @@ pub mod parse_worker {
// SAFETY: `result` is a valid heap pointer with `task` at the given offset;
// ownership transfers to the mini event loop which frees it after `on_complete_mini`.
unsafe {
mini.enqueue_task_concurrent_with_extra_ctx::<Result, BundleV2<'static>>(
bun_event_loop::MiniEventLoop::MiniEventLoop::enqueue_task_concurrent_with_extra_ctx::<Result, BundleV2<'static>>(
&raw const **mini,
result,
on_complete_mini,
offset_of!(Result, task),
Expand Down
3 changes: 2 additions & 1 deletion src/bundler/ServerComponentParseTask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ fn task_callback_wrap(thread_pool_task: *mut ThreadPoolTask) {
// SAFETY: `result` is a freshly Box-leaked `parse_task::Result` (above) and
// `offset_of!(parse_task::Result, task)` is the intrusive task field within it.
unsafe {
mini.enqueue_task_concurrent_with_extra_ctx::<parse_task::Result, BundleV2<'static>>(
bun_event_loop::MiniEventLoop::MiniEventLoop::enqueue_task_concurrent_with_extra_ctx::<parse_task::Result, BundleV2<'static>>(
&raw const **mini,
result,
on_complete_mini,
offset_of!(parse_task::Result, task),
Expand Down
6 changes: 4 additions & 2 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4343,7 +4343,8 @@ pub mod bv2_impl {
// SAFETY: `load` is a valid &mut for the duration of the enqueue;
// the mini loop dispatches `on_load_mini` on the bundler thread.
unsafe {
mini.enqueue_task_concurrent_with_extra_ctx::<jsc_api::JSBundler::Load, BundleV2<'static>>(
bun_event_loop::MiniEventLoop::MiniEventLoop::enqueue_task_concurrent_with_extra_ctx::<jsc_api::JSBundler::Load, BundleV2<'static>>(
&raw const **mini,
std::ptr::from_mut(load),
on_load_mini,
core::mem::offset_of!(jsc_api::JSBundler::Load, task),
Expand Down Expand Up @@ -4377,7 +4378,8 @@ pub mod bv2_impl {
// SAFETY: `resolve` is a valid &mut for the duration of the enqueue;
// the mini loop dispatches `on_resolve_mini` on the bundler thread.
unsafe {
mini.enqueue_task_concurrent_with_extra_ctx::<jsc_api::JSBundler::Resolve, BundleV2<'static>>(
bun_event_loop::MiniEventLoop::MiniEventLoop::enqueue_task_concurrent_with_extra_ctx::<jsc_api::JSBundler::Resolve, BundleV2<'static>>(
&raw const **mini,
std::ptr::from_mut(resolve),
on_resolve_mini,
core::mem::offset_of!(jsc_api::JSBundler::Resolve, task),
Expand Down
49 changes: 33 additions & 16 deletions src/event_loop/MiniEventLoop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl MiniEventLoop {
/// returning accessor is intentionally **not** provided: `UwsLoop::tick()`
/// fires FilePoll callbacks which re-enter this struct via the
/// `EventLoopCtx` vtable (`platform_event_loop`) and via
/// `EventLoopHandle::Mini` (e.g. `enqueue_task_concurrent` → `wakeup()`),
/// `EventLoopHandle::Mini` (e.g. `enqueue_task_concurrent`, which wakes it),
/// so a held `&mut UwsLoop` across `.tick()` would alias. The loop is also
/// a C-owned handle whose internals are mutated by uSockets itself. All
/// access goes through the raw pointer instead.
Expand Down Expand Up @@ -383,23 +383,43 @@ impl MiniEventLoop {
}
}

/// `task` must outlive the queued work item; ownership of the intrusive
/// node stays with the caller until the callback runs.
pub fn enqueue_task_concurrent(&mut self, task: NonNull<AnyTaskWithExtraContext>) {
self.concurrent_tasks.push(task);
// SAFETY: see `loop_ptr()` invariant.
unsafe { (*self.loop_ptr()).wakeup() };
/// Post `task` to the thread that owns this loop and wake it. Any thread.
///
/// Raw pointer, not a receiver: the owner may run the task and free the
/// loop as soon as it is queued (`Bun.build` boxes one loop per pass), i.e.
/// before this returns. A reference argument would assert `*this` for the
/// whole call; here nothing reads `*this` after the push.
///
/// # Safety
/// `this` must point to a live `MiniEventLoop` when called; its owner may
/// free it once the task is queued. `task` must outlive the queued work
/// item; ownership of the intrusive node stays with the caller until the
/// callback runs.
Comment thread
robobun marked this conversation as resolved.
pub unsafe fn enqueue_task_concurrent(
this: *const Self,
task: NonNull<AnyTaskWithExtraContext>,
) {
// SAFETY: `*this` is live until `push_raw` publishes `task` (fn contract)
// and is not touched after; `loop_` outlives this struct (see `loop_ptr`)
// and `us_wakeup_loop` is thread-safe.
unsafe {
let loop_ = (*this).loop_;
ConcurrentTaskQueue::push_raw(&raw const (*this).concurrent_tasks, task);
bun_uws::us_wakeup_loop(loop_);
}
}

/// The caller supplies `field_offset = core::mem::offset_of!(C, <field>)` of the
/// embedded `AnyTaskWithExtraContext`.
/// Initialize the `AnyTaskWithExtraContext` embedded in `*ctx` at
/// `field_offset = core::mem::offset_of!(C, <field>)`, then post it via
/// [`enqueue_task_concurrent`](Self::enqueue_task_concurrent).
Comment thread
robobun marked this conversation as resolved.
///
/// # Safety
/// `this` as for [`enqueue_task_concurrent`](Self::enqueue_task_concurrent).
/// `field_offset == offset_of!(C, <field>)` where `<field>: AnyTaskWithExtraContext`,
/// and `ctx` is non-null and outlives the queued task (intrusive node; ownership stays
/// with caller).
pub unsafe fn enqueue_task_concurrent_with_extra_ctx<C, P>(
&mut self,
this: *const Self,
ctx: *mut C,
callback: fn(*mut C, *mut P),
field_offset: usize,
Expand All @@ -409,12 +429,9 @@ impl MiniEventLoop {
// SAFETY: `task` points at a properly aligned `AnyTaskWithExtraContext` field of `*ctx`.
unsafe { task.write(New::<C, P>::init(ctx, callback)) };

// SAFETY: `task` was just initialized above and is non-null (derived from `ctx`).
self.concurrent_tasks
.push(unsafe { NonNull::new_unchecked(task) });

// SAFETY: see `loop_ptr()` invariant.
unsafe { (*self.loop_ptr()).wakeup() };
// SAFETY: `task` was just initialized above and is non-null (derived from
// `ctx`); `this` is forwarded under the same contract.
unsafe { Self::enqueue_task_concurrent(this, NonNull::new_unchecked(task)) };
}
}

Expand Down
11 changes: 5 additions & 6 deletions src/jsc/VmHandle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use bun_threading::{Condvar, Mutex};
use crate::event_loop::EventLoop;
use crate::virtual_machine::VirtualMachine;
use bun_event_loop::ConcurrentTask::ConcurrentTask as ConcurrentTaskItem;
use bun_event_loop::MiniEventLoop::MiniEventLoop;

#[repr(u8)]
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
Expand Down Expand Up @@ -599,7 +600,7 @@ pub enum ConcurrentPoster {
/// Erased handle of the JS loop's VM (obtained from the `EventLoopHandle`
/// itself, so it is correct whichever thread constructs the poster).
Js(bun_event_loop::JsPoster),
Mini(bun_ptr::BackRef<bun_event_loop::MiniEventLoop::MiniEventLoop, bun_ptr::Mut>),
Mini(bun_ptr::BackRef<MiniEventLoop, bun_ptr::Mut>),
}

impl ConcurrentPoster {
Expand Down Expand Up @@ -650,11 +651,9 @@ impl ConcurrentPoster {
) {
match self {
ConcurrentPoster::Mini(mini) => {
let mut mini = *mini;
// SAFETY: per `EventLoopHandle::Mini` invariant — the mini loop is
// alive for as long as work it created runs; its concurrent queue
// push is thread-safe.
unsafe { mini.get_mut() }.enqueue_task_concurrent(task);
// SAFETY: `EventLoopHandle::Mini` invariant — the loop is alive while
// work it created runs, and this post is that work's last step.
unsafe { MiniEventLoop::enqueue_task_concurrent(mini.as_const_ptr(), task) };
}
ConcurrentPoster::Js(..) => debug_assert!(false, "post_mini on a Js poster"),
}
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/api/JSBundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1535,7 +1535,8 @@ pub mod js_bundler {
bun_event_loop::AnyEventLoop::Mini(mini) => {
// `mini.enqueueTaskConcurrentWithExtraCtx(
// Load, BundleV2, this, BundleV2.onNotifyDeferMini, .task)`
mini.enqueue_task_concurrent_with_extra_ctx::<Load, BundleV2<'static>>(
bun_event_loop::MiniEventLoop::MiniEventLoop::enqueue_task_concurrent_with_extra_ctx::<Load, BundleV2<'static>>(
&raw const **mini,
std::ptr::from_mut::<Load>(self),
on_notify_defer_mini_wrap,
core::mem::offset_of!(Load, task),
Expand Down
5 changes: 3 additions & 2 deletions src/runtime/shell/builtin/yes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::shell::states::cmd::Exec;
use crate::shell::yield_::Yield;

use bun_event_loop::ConcurrentTask::AutoDeinit;
use bun_event_loop::MiniEventLoop::MiniEventLoop;
use bun_event_loop::{EventLoopTask, TaskTag, Taskable, task_tag};

#[derive(Clone, Copy, PartialEq, Eq, Default)]
Expand Down Expand Up @@ -226,7 +227,7 @@ impl YesTask {
// Same-thread bounce on the loop's own thread: always accepted.
let _ = owner.js_poster().post(ct);
}
EventLoopHandle::Mini(mut mini) => {
EventLoopHandle::Mini(mini) => {
(*mini.loop_).tick();
let at =
core::ptr::NonNull::new_unchecked(match &mut (*this).concurrent_task {
Expand All @@ -235,7 +236,7 @@ impl YesTask {
}
EventLoopTask::Js(_) => unreachable!(),
});
mini.get_mut().enqueue_task_concurrent(at);
MiniEventLoop::enqueue_task_concurrent(mini.as_const_ptr(), at);
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions src/spawn/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,7 @@ pub mod waiter_thread_posix {
use super::*;
use bun_event_loop::AnyTaskWithExtraContext::{AnyTaskWithExtraContext, New as AnyTaskNew};
use bun_event_loop::ConcurrentTask::{ConcurrentTask, Task, TaskTag};
use bun_event_loop::MiniEventLoop::MiniEventLoop;
use bun_event_loop::task_tag;
use bun_threading::UnboundedQueue;

Expand Down Expand Up @@ -1198,19 +1199,23 @@ pub mod waiter_thread_posix {
}
}
}
EventLoopHandle::Mini(mut mini) => {
EventLoopHandle::Mini(mini) => {
let out = ResultTaskMini::<T>::new(ResultTaskMini {
result,
subprocess: process,
task: AnyTaskWithExtraContext::default(),
});
// SAFETY: `out` just produced by heap::alloc — non-null.
// SAFETY: `out` just produced by heap::alloc — non-null;
// `mini` is the handle's live loop (`EventLoopHandle::Mini`
// invariant) and this post is the last thing the waiter
// thread does with it.
unsafe {
(*out).task = AnyTaskNew::<ResultTaskMini<T>, ()>::init(
out,
ResultTaskMini::<T>::run_from_main_thread_mini,
);
mini.get_mut().enqueue_task_concurrent(
MiniEventLoop::enqueue_task_concurrent(
mini.as_const_ptr(),
core::ptr::NonNull::new_unchecked(core::ptr::addr_of_mut!(
(*out).task
)),
Expand Down
6 changes: 6 additions & 0 deletions src/threading/WaitGroup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ mod tests {

// After `wait()` returns the caller may drop the `WaitGroup`; `finish()`
// must therefore not touch `self` once it has published `raw_count == 0`.
// Miri (Tree Borrows) currently reports this as violated: the `&self` of
// `finish` / `Mutex::unlock` still covers the group when the waiter frees it.
Comment thread
robobun marked this conversation as resolved.
#[cfg_attr(
miri,
ignore = "finish(&self) is still on the finisher's stack when wait() returns"
)]
#[test]
fn wait_returning_means_finish_is_done_with_self() {
for _ in 0..10_000 {
Expand Down
Loading
Loading