fix(linter): prevent unsound use of Allocator across threads

Author: overlookmotel

crates/oxc_language_server/src/linter/isolated_lint_handler.rs

@@ -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))
@@ -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>>> {

crates/oxc_linter/src/service/mod.rs

@@ -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)
     }
@@ -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>> {

crates/oxc_linter/src/service/runtime.rs

@@ -173,6 +173,61 @@ 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: Not safe by itself. `MessageCloner` handles synchronization with a `Mutex`.
+    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,
@@ -588,12 +643,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},
@@ -614,6 +669,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| {
@@ -655,7 +713,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
@@ -742,13 +800,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| {
@@ -773,13 +833,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)
                                 }),
                             );
                         }

crates/oxc_linter/src/tester.rs

@@ -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(
@@ -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;