Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ impl IsolatedLintHandler {
return None;
}

let allocator = Allocator::default();
let mut allocator = Allocator::default();

Some(self.lint_path(&allocator, &path, content).map_or(vec![], |errors| {
Some(self.lint_path(&mut allocator, &path, content).map_or(vec![], |errors| {
let mut diagnostics: Vec<DiagnosticReport> = errors
.iter()
.map(|e| message_with_position_to_lsp_diagnostic_report(e, uri))
Expand Down Expand Up @@ -143,7 +143,7 @@ impl IsolatedLintHandler {

fn lint_path<'a>(
&mut self,
allocator: &'a Allocator,
allocator: &'a mut Allocator,
path: &Path,
source_text: Option<String>,
) -> Option<Vec<MessageWithPosition<'a>>> {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl LintService {
#[cfg(feature = "language_server")]
pub fn run_source<'a>(
&mut self,
allocator: &'a oxc_allocator::Allocator,
allocator: &'a mut oxc_allocator::Allocator,
) -> Vec<crate::MessageWithPosition<'a>> {
self.runtime.run_source(allocator)
}
Expand All @@ -105,7 +105,7 @@ impl LintService {
#[cfg(test)]
pub(crate) fn run_test_source<'a>(
&mut self,
allocator: &'a oxc_allocator::Allocator,
allocator: &'a mut oxc_allocator::Allocator,
check_syntax_errors: bool,
tx_error: &DiagnosticSender,
) -> Vec<crate::Message<'a>> {
Expand Down
89 changes: 80 additions & 9 deletions crates/oxc_linter/src/service/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,72 @@ impl RuntimeFileSystem for OsFileSystem {
}
}

/// [`MessageCloner`] is a wrapper around an `&Allocator` which allows it to be safely shared across threads,
/// in order to clone [`Message`]s into it.
///
/// `Allocator` is not thread safe (it is not `Sync`), so cannot be shared across threads.
/// It would be undefined behavior to allocate into an `Allocator` from multiple threads simultaneously.
///
/// `MessageCloner` ensures only one thread at a time can utilize the `Allocator`, by taking an
/// exclusive `&mut Allocator` to start with, and synchronising access to the `Allocator` with a `Mutex`.
///
/// This type is wrapped in a module so that other code cannot access the inner `UnsafeAllocatorRef`
/// directly, and must go via the [`MessageCloner::clone_message`] method.
mod message_cloner {
use std::sync::Mutex;

use oxc_allocator::{Allocator, CloneIn};

use crate::Message;

/// Unsafe wrapper around an `&Allocator` which makes it `Send`.
struct UnsafeAllocatorRef<'a>(&'a Allocator);

// SAFETY: It is sound to implement `Send` for `UnsafeAllocatorRef` because:
// * The only way to construct an `UnsafeAllocatorRef` is via `MessageCloner::new`, which takes
// an exclusive `&mut Allocator`, ensuring no other references to the same `Allocator` exist.
// * The lifetime `'a` ensures that the reference to the `Allocator` cannot outlive the original
// mutable borrow, preventing aliasing or concurrent mutation.
// * All access to the `Allocator` via `UnsafeAllocatorRef` is synchronized by a `Mutex` inside
// `MessageCloner`, so only one thread can access the allocator at a time.
// * The module encapsulation prevents direct access to `UnsafeAllocatorRef`, so it cannot be
// misused outside of the intended, synchronized context.
//
// Therefore, although `Allocator` is not `Sync`, it is safe to send `UnsafeAllocatorRef` between
// threads as long as it is only accessed via the `Mutex` in `MessageCloner`.
unsafe impl Send for UnsafeAllocatorRef<'_> {}

/// Wrapper around an [`Allocator`] which allows safely using it on multiple threads to
/// clone [`Message`]s into.
pub struct MessageCloner<'a>(Mutex<UnsafeAllocatorRef<'a>>);

impl<'a> MessageCloner<'a> {
/// Wrap an [`Allocator`] in a [`MessageCloner`].
///
/// This method takes a `&mut Allocator`, to ensure that no other references to the `Allocator`
/// can exist, which guarantees no other threads can allocate with the `Allocator` while this
/// `MessageCloner` exists.
#[inline]
#[expect(clippy::needless_pass_by_ref_mut)]
pub fn new(allocator: &'a mut Allocator) -> Self {
Self(Mutex::new(UnsafeAllocatorRef(allocator)))
}

/// Clone a [`Message`] into the [`Allocator`] held by this [`MessageCloner`].
///
/// # Panics
/// Panics if the underlying `Mutex` is poisoned.
pub fn clone_message(&self, message: &Message) -> Message<'a> {
// Obtain an exclusive lock on the `Mutex` during `clone_in` operation,
// to ensure no other thread can be simultaneously using the `Allocator`
let guard = self.0.lock().unwrap();
let allocator = guard.0;
message.clone_in(allocator)
}
}
}
use message_cloner::MessageCloner;

impl Runtime {
pub(super) fn new(
linter: Linter,
Expand Down Expand Up @@ -588,12 +654,12 @@ impl Runtime {
#[cfg(feature = "language_server")]
pub(super) fn run_source<'a>(
&mut self,
allocator: &'a oxc_allocator::Allocator,
allocator: &'a mut oxc_allocator::Allocator,
) -> Vec<MessageWithPosition<'a>> {
use oxc_allocator::CloneIn;
use oxc_data_structures::rope::Rope;
use std::sync::Mutex;

use oxc_data_structures::rope::Rope;

use crate::{
FixWithPosition,
fixer::{Fix, PossibleFixesWithPosition},
Expand All @@ -614,6 +680,9 @@ impl Runtime {
}
}

// Wrap allocator in `MessageCloner` so can clone `Message`s into it
let message_cloner = MessageCloner::new(allocator);

let messages = Mutex::new(Vec::<MessageWithPosition<'a>>::new());
let (sender, _receiver) = mpsc::channel();
rayon::scope(|scope| {
Expand Down Expand Up @@ -655,7 +724,7 @@ impl Runtime {

messages.lock().unwrap().extend(section_messages.iter().map(
|message| {
let message = message.clone_in(allocator);
let message = message_cloner.clone_message(message);

let labels = &message.error.labels.clone().map(|labels| {
labels
Expand Down Expand Up @@ -742,13 +811,15 @@ impl Runtime {
#[cfg(test)]
pub(super) fn run_test_source<'a>(
&mut self,
allocator: &'a Allocator,
allocator: &'a mut Allocator,
check_syntax_errors: bool,
tx_error: &DiagnosticSender,
) -> Vec<Message<'a>> {
use oxc_allocator::CloneIn;
use std::sync::Mutex;

// Wrap allocator in `MessageCloner` so can clone `Message`s into it
let message_cloner = MessageCloner::new(allocator);

let messages = Mutex::new(Vec::<Message<'a>>::new());
rayon::scope(|scope| {
self.resolve_modules(scope, check_syntax_errors, tx_error, |me, mut module| {
Expand All @@ -773,13 +844,13 @@ impl Runtime {
.map(|err| Message::new(err, PossibleFixes::None))
.collect(),
}
.into_iter()
.map(|mut message| {
.iter_mut()
.map(|message| {
if section.source.start != 0 {
message.move_offset(section.source.start)
.move_fix_offset(section.source.start);
}
message.clone_in(allocator)
message_cloner.clone_message(message)
}),
);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl Tester {
fix_kind: ExpectFixKind,
fix_index: u8,
) -> TestResult {
let allocator = Allocator::default();
let mut allocator = Allocator::default();
let rule = self.find_rule().read_json(rule_config.unwrap_or_default());
let mut external_plugin_store = ExternalPluginStore::default();
let linter = Linter::new(
Expand Down Expand Up @@ -554,7 +554,7 @@ impl Tester {
.with_paths(paths);

let (sender, _receiver) = mpsc::channel();
let result = lint_service.run_test_source(&allocator, false, &sender);
let result = lint_service.run_test_source(&mut allocator, false, &sender);

if result.is_empty() {
return TestResult::Passed;
Expand Down
Loading