Skip to content

refactor(napi/oxlint): move creating JS buffer into separate function #13133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Aug 16, 2025
Merged

refactor(napi/oxlint): move creating JS buffer into separate function #13133

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
145 changes: 81 additions & 64 deletions napi/oxlint2/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,70 +75,9 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb {

let (tx, rx) = channel();

// Each buffer is sent over to JS only once.
// JS side stores them in an array, and holds them until process ends.
// A flag in `FixedSizeAllocatorMetadata` records whether the buffer has already been transferred
// to JS or not. If it hasn't, send it. Otherwise, just send the ID of the buffer which is the
// index of that buffer in the array on JS side, and JS side will get the buffer from the array.
// This means there's only ever 1 instance of a buffer on Rust side, and 1 on JS side,
// which makes it simpler to avoid use-after-free or double-free problems.

// SAFETY: This crate enables the `fixed_size` feature on `oxc_allocator`, so all AST `Allocator`s
// are created via `FixedSizeAllocator`. We only create an immutable ref from this pointer.
let metadata_ptr = unsafe { allocator.fixed_size_metadata_ptr() };
let (buffer_id, already_sent_to_js) = {
// SAFETY: Fixed-size allocators always have a valid `FixedSizeAllocatorMetadata`
// stored at the pointer returned by `Allocator::fixed_size_metadata_ptr`.
let metadata = unsafe { metadata_ptr.as_ref() };
// TODO: Is `Ordering::SeqCst` excessive here?
let already_sent_to_js = metadata.is_double_owned.swap(true, Ordering::SeqCst);

(metadata.id, already_sent_to_js)
};

let buffer = if already_sent_to_js {
// Buffer has already been sent to JS. Don't send it again.
None
} else {
// Buffer has not already been sent to JS. Send it.

// Get pointer to start of allocator chunk.
// Note: `Allocator::data_ptr` would not provide the right pointer, because source text
// gets written to start of the allocator chunk, and `data_ptr` gets moved to after it.
// SAFETY: Fixed-size allocators have their chunk aligned on `BLOCK_ALIGN`,
// and size less than `BLOCK_ALIGN`. So we can get pointer to start of `Allocator` chunk
// by rounding down to next multiple of `BLOCK_ALIGN`. That can't go out of bounds of
// the backing allocation.
let chunk_ptr = unsafe {
let ptr = metadata_ptr.cast::<u8>();
let offset = ptr.as_ptr() as usize % BLOCK_ALIGN;
ptr.sub(offset)
};

// SAFETY:
// Range of memory starting at `chunk_ptr` and encompassing `BUFFER_SIZE` is all within
// the allocation backing the `Allocator`.
//
// We can't prove that no mutable references to data in the buffer exist,
// but there shouldn't be any, because linter doesn't mutate the AST.
// Anyway, I (@overlookmotel) am not sure if the aliasing rules apply to code in another
// language. Probably not, as JS code is outside the domain of the "Rust abstract machine".
// As long as we don't mutate data in the buffer on JS side, it should be fine.
//
// On the other side, while many immutable references to data in the buffer exist
// (`AstKind`s for every AST node), JS side does not mutate the data in the buffer,
// so that shouldn't break the guarantees of `&` references.
//
// This is all a bit wavy, but such is the way with sharing memory outside of Rust.
let buffer = unsafe {
Uint8Array::with_external_data(
chunk_ptr.as_ptr(),
BUFFER_SIZE,
move |_ptr, _len| free_fixed_size_allocator(metadata_ptr),
)
};
Some(buffer)
};
// SAFETY: This crate enables the `fixed_size` feature on `oxc_allocator`,
// so all AST `Allocator`s are created via `FixedSizeAllocator`
let (buffer_id, buffer) = unsafe { get_buffer(allocator) };

// Send data to JS
let status = cb.call_with_return_value(
Expand Down Expand Up @@ -169,6 +108,84 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb {
})
}

/// Get buffer ID of the `Allocator` and, if it hasn't already been sent to JS,
/// create a `Uint8Array` referencing the `Allocator`'s memory.
///
/// Each buffer is sent over to JS only once.
/// JS side stores them in an array (indexed by buffer ID), and holds them until process ends.
/// This means there's only ever 1 instance of a buffer on Rust side, and 1 on JS side,
/// which makes it simpler to avoid use-after-free or double-free problems.
///
/// So only create a `Uint8Array` if it's not already sent to JS.
///
/// Whether the buffer has already been sent to JS is tracked by a flag in `FixedSizeAllocatorMetadata`,
/// which is stored in memory backing the `Allocator`.
///
/// # SAFETY
/// `allocator` must have been created via `FixedSizeAllocator`
unsafe fn get_buffer(
allocator: &Allocator,
) -> (
u32, // Buffer ID
Option<Uint8Array>, // Buffer, if not already sent to JS
) {
// SAFETY: Caller guarantees `Allocator` was created by a `FixedSizeAllocator`.
// We only create an immutable ref from this pointer.
let metadata_ptr = unsafe { allocator.fixed_size_metadata_ptr() };
// SAFETY: Fixed-size allocators always have a valid `FixedSizeAllocatorMetadata`
// stored at the pointer returned by `Allocator::fixed_size_metadata_ptr`.
let metadata = unsafe { metadata_ptr.as_ref() };

let buffer_id = metadata.id;

// Get whether this buffer has already been sent to JS
// TODO: Is `SeqCst` excessive here?
let already_sent_to_js = metadata.is_double_owned.swap(true, Ordering::SeqCst);

// If buffer has already been sent to JS, don't send it again
if already_sent_to_js {
return (buffer_id, None);
}

// Buffer has not already been sent to JS. Send it.

// Get pointer to start of allocator chunk.
// Note: `Allocator::data_ptr` would not provide the right pointer, because source text
// gets written to start of the allocator chunk, and `data_ptr` gets moved to after it.
// SAFETY: Fixed-size allocators have their chunk aligned on `BLOCK_ALIGN`,
// and size less than `BLOCK_ALIGN`. So we can get pointer to start of `Allocator` chunk
// by rounding down to next multiple of `BLOCK_ALIGN`. That can't go out of bounds of
// the backing allocation.
let chunk_ptr = unsafe {
let ptr = metadata_ptr.cast::<u8>();
let offset = ptr.as_ptr() as usize % BLOCK_ALIGN;
ptr.sub(offset)
};

// SAFETY:
// Range of memory starting at `chunk_ptr` and encompassing `BUFFER_SIZE` is all within
// the allocation backing the `Allocator`.
//
// We can't prove that no mutable references to data in the buffer exist,
// but there shouldn't be any, because linter doesn't mutate the AST.
// Anyway, I (@overlookmotel) am not sure if the aliasing rules apply to code in another
// language. Probably not, as JS code is outside the domain of the "Rust abstract machine".
// As long as we don't mutate data in the buffer on JS side, it should be fine.
//
// On the other side, while many immutable references to data in the buffer exist
// (`AstKind`s for every AST node), JS side does not mutate the data in the buffer,
// so that shouldn't break the guarantees of `&` references.
//
// This is all a bit wavy, but such is the way with sharing memory outside of Rust.
let buffer = unsafe {
Uint8Array::with_external_data(chunk_ptr.as_ptr(), BUFFER_SIZE, move |_ptr, _len| {
free_fixed_size_allocator(metadata_ptr);
})
};

(buffer_id, Some(buffer))
}

#[expect(clippy::allow_attributes)]
#[allow(clippy::trailing_empty_array, clippy::unused_async)] // https://github.com/napi-rs/napi-rs/issues/2758
#[napi]
Expand Down
Loading