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
12 changes: 7 additions & 5 deletions src/jsc/JSSecrets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,16 @@ impl crate::JobContext for SecretsJob {
type OffThread = Self;
type Js = Strong;

fn run(
this: &mut Self,
unsafe fn run(
this: *mut Self,
vm: &crate::vm_handle::Borrow,
done: crate::Completion<Self>,
) -> Option<crate::Completion<Self>> {
// SAFETY: the creating global, alive under the borrow; C++ only threads it through.
let global = unsafe { this.global.under_borrow(vm) };
Bun__SecretsJobOptions__runTask(SecretsJobOptions::opaque_mut(this.options.0), global);
// SAFETY: fn contract; both fields are copied out of the live job. The
// global is the creating one, alive under the borrow; C++ only threads
// it through.
let (options, global) = unsafe { ((*this).options.0, (*this).global.under_borrow(vm)) };
Bun__SecretsJobOptions__runTask(SecretsJobOptions::opaque_mut(options), global);
Some(done)
}

Expand Down
22 changes: 17 additions & 5 deletions src/jsc/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,19 @@ pub trait JobContext: Sized + 'static {
/// Return `done` to complete now; keep it (e.g. across async I/O that
/// finishes on another thread) and call [`Completion::finish`] later to
/// complete then. Work that outlives this call runs under no borrow and
/// must touch only `off`.
fn run(
off: &mut Self::OffThread,
/// must touch only the off-thread part.
///
/// `off` is a pointer, not `&mut`: a body that keeps `done` hands the job
/// on before returning, and the JS thread then frees it through the job's
/// own pointer (`Job::complete`), which is UB while a reference argument
/// is still protected here. Such a body reborrows only for work that ends
/// before the hand-over, which is its last access.
///
/// # Safety
/// `off` is the live job's off-thread part and nothing else touches it
/// until this returns `Some(done)` or the body hands the job on.
Comment on lines +241 to +251

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +241 to +251

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

unsafe fn run(
off: *mut Self::OffThread,
vm: &Borrow,
done: Completion<Self>,
) -> Option<Completion<Self>>;
Expand Down Expand Up @@ -388,7 +398,9 @@ impl<C: JobContext> Job<C> {
return done.finish();
};
// SAFETY: as above; the borrow keeps the VM (and any JsPtr target) alive.
if let Some(done) = C::run(unsafe { &mut (*this).off }, &vm, done) {
// On `None` the job may already be freed; nothing below touches it
// (`vm` is released through our own `handle` clone).
Comment on lines +401 to +402

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

if let Some(done) = unsafe { C::run(&raw mut (*this).off, &vm, done) } {
drop(vm);
done.finish();
}
Expand Down Expand Up @@ -541,7 +553,7 @@ pub enum Never {}
impl JobContext for Never {
type OffThread = ();
type Js = ();
fn run(_: &mut (), _: &Borrow, done: Completion<Self>) -> Option<Completion<Self>> {
unsafe fn run(_: *mut (), _: &Borrow, done: Completion<Self>) -> Option<Completion<Self>> {
Some(done)
}
fn then(_: (), _: (), _: &JsThread<'_>) -> JsResult<()> {
Expand Down
8 changes: 5 additions & 3 deletions src/runtime/api/Archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,12 +687,14 @@ pub struct AsyncTask<C: TaskContext>(core::marker::PhantomData<C>);
impl<C: TaskContext> bun_jsc::JobContext for AsyncTask<C> {
type OffThread = C;
type Js = JSPromiseStrong;
fn run(
ctx: &mut C,
unsafe fn run(
ctx: *mut C,
_vm: &bun_jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
ctx.run();
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*ctx).run() };
Some(done)
}
fn then(mut ctx: C, mut promise: JSPromiseStrong, cx: &bun_jsc::JsThread<'_>) -> JsResult<()> {
Expand Down
57 changes: 33 additions & 24 deletions src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2852,54 +2852,63 @@ pub mod JSZstd {
pub error_message: Option<&'static [u8]>,
}

impl jsc::JobContext for ZstdJob {
type OffThread = Self;
type Js = jsc::JSPromiseStrong;
impl ZstdJob {
/// Pool thread: fills `output`, or sets `error_message`.
fn run(&mut self) {
let input = self.buffer.slice();

fn run(
this: &mut Self,
_vm: &jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
let input = this.buffer.slice();

if this.is_compress {
if self.is_compress {
let max_size = bun_zstd::compress_bound(input.len());
// Surface OOM as a rejected promise instead of aborting. The
// zero-fill is output-irrelevant (zstd overwrites the prefix it reports).
let mut output: Vec<u8> = Vec::new();
if output.try_reserve_exact(max_size).is_err() {
this.error_message = Some(b"Out of memory");
return Some(done);
self.error_message = Some(b"Out of memory");
return;
}
output.resize(max_size, 0);
this.output = output;
self.output = output;

this.output = match bun_zstd::compress(&mut this.output, input, Some(this.level)) {
self.output = match bun_zstd::compress(&mut self.output, input, Some(self.level)) {
bun_zstd::Result::Success(size) => 'blk: {
if size < this.output.len() {
let mut out = core::mem::take(&mut this.output);
if size < self.output.len() {
let mut out = core::mem::take(&mut self.output);
out.truncate(size);
out.shrink_to_fit();
break 'blk out;
}
break 'blk core::mem::take(&mut this.output);
break 'blk core::mem::take(&mut self.output);
}
bun_zstd::Result::Err(err) => {
this.output = Vec::new();
this.error_message = Some(err);
return Some(done);
self.output = Vec::new();
self.error_message = Some(err);
return;
}
};
} else {
this.output = match bun_zstd::decompress_alloc(input) {
self.output = match bun_zstd::decompress_alloc(input) {
Ok(v) => v,
Err(_) => {
this.error_message = Some(b"Decompression failed");
return Some(done);
self.error_message = Some(b"Decompression failed");
return;
}
};
}
}
}

impl jsc::JobContext for ZstdJob {
type OffThread = Self;
type Js = jsc::JSPromiseStrong;

unsafe fn run(
this: *mut Self,
_vm: &jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*this).run() };
Some(done)
}

Expand Down
8 changes: 5 additions & 3 deletions src/runtime/api/JSTranspiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,12 +677,14 @@ pub(crate) struct TransformJs {
impl jsc::JobContext for TransformTask {
type OffThread = Self;
type Js = TransformJs;
fn run(
this: &mut Self,
unsafe fn run(
this: *mut Self,
vm: &jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
TransformTask::run(this, vm);
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*this).run(vm) };
Some(done)
}
fn then(mut this: Self, mut js: TransformJs, cx: &jsc::JsThread<'_>) -> JsResult<()> {
Expand Down
33 changes: 21 additions & 12 deletions src/runtime/api/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,25 +243,34 @@ impl WalkTaskErr {
}
}

impl WalkTask {
/// Pool thread: runs the walk, recording a failure in `err`.
fn run(&mut self) {
let result = match self.walker.walk() {
Ok(r) => r,
Err(err) => {
self.err = Some(WalkTaskErr::Unknown(err.into()));
return;
}
};
if let bun_sys::Result::Err(err) = result {
self.err = Some(WalkTaskErr::Syscall(err));
}
}
}

impl JobContext for WalkTask {
type OffThread = Self;
type Js = WalkJs;

fn run(
this: &mut Self,
unsafe fn run(
this: *mut Self,
_vm: &bun_jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
let result = match this.walker.walk() {
Ok(r) => r,
Err(err) => {
this.err = Some(WalkTaskErr::Unknown(err.into()));
return Some(done);
}
};
if let bun_sys::Result::Err(err) = result {
this.err = Some(WalkTaskErr::Syscall(err));
}
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*this).run() };
Some(done)
}

Expand Down
41 changes: 25 additions & 16 deletions src/runtime/crypto/PBKDF2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,30 +266,39 @@ pub(crate) struct Pbkdf2Job {
pub err: bool,
}

impl JobContext for Pbkdf2Job {
type OffThread = Self;
type Js = JSPromiseStrong;

fn run(
this: &mut Self,
_vm: &bun_jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
let len = usize::try_from(this.pbkdf2.length).expect("int cast");
impl Pbkdf2Job {
/// Pool thread: derives into `output`, or sets `err`.
fn run(&mut self) {
let len = usize::try_from(self.pbkdf2.length).expect("int cast");
// `Vec` allocation aborts on OOM; use try_reserve to surface an error instead.
let mut buf = Vec::new();
if buf.try_reserve_exact(len).is_err() {
this.err = true;
return Some(done);
self.err = true;
return;
}
buf.resize(len, 0);
this.output = buf;
self.output = buf;

if !this.pbkdf2.run(&mut this.output) {
this.err = true;
if !self.pbkdf2.run(&mut self.output) {
self.err = true;
boringssl::ERR_clear_error();
this.output = Vec::new();
self.output = Vec::new();
}
}
}

impl JobContext for Pbkdf2Job {
type OffThread = Self;
type Js = JSPromiseStrong;

unsafe fn run(
this: *mut Self,
_vm: &bun_jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*this).run() };
Some(done)
}

Expand Down
8 changes: 5 additions & 3 deletions src/runtime/crypto/PasswordObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,14 @@ impl<Op: PasswordOp> Drop for PasswordJob<Op> {
impl<Op: PasswordOp> bun_jsc::JobContext for PasswordJob<Op> {
type OffThread = Self;
type Js = JSPromiseStrong;
fn run(
this: &mut Self,
unsafe fn run(
this: *mut Self,
_vm: &bun_jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
this.value = Some(this.op.compute(&this.password));
// SAFETY: fn contract; the job is not handed on, so the reborrows are
// exclusive for the statement.
unsafe { (*this).value = Some((*this).op.compute(&(*this).password)) };
Some(done)
}
fn then(
Expand Down
8 changes: 5 additions & 3 deletions src/runtime/dns_jsc/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,12 +1010,14 @@ pub mod get_addr_info_request {
impl bun_jsc::JobContext for LibcLookup {
type OffThread = Self;
type Js = LibcRequest;
fn run(
this: &mut Self,
unsafe fn run(
this: *mut Self,
_vm: &bun_jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
this.backend.run();
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*this).backend.run() };
Some(done)
}
fn then(
Expand Down
8 changes: 5 additions & 3 deletions src/runtime/image/Image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1465,12 +1465,14 @@ impl Drop for PendingTask {
impl jsc::JobContext for PipelineTask {
type OffThread = Self;
type Js = PipelineJs;
fn run(
this: &mut Self,
unsafe fn run(
this: *mut Self,
_vm: &jsc::vm_handle::Borrow,
done: bun_jsc::Completion<Self>,
) -> Option<bun_jsc::Completion<Self>> {
this.run();
// SAFETY: fn contract; the job is not handed on, so the reborrow is
// exclusive for the call.
unsafe { (*this).run() };
Some(done)
}
fn then(this: Self, js: PipelineJs, cx: &jsc::JsThread<'_>) -> jsc::JsResult<()> {
Expand Down
Loading
Loading