fix(linter): OOM problems with custom plugins (#17082)

overlookmotel · camc314 · web-flow · commit fb9e193f991b · 2025-12-18T23:01:11.000Z
This PR fixes OOM (Out of Memory) problems when using JS plugins
together with the import plugin for cross-module analysis.

Previously, a stopgap solution simply refused to run when &gt;10,000 files
were linted with both features enabled.

This PR implements a real fix using a deferred cloning strategy:
standard allocators are used for parsing/linting (efficient, dynamic
sizing), and the AST is only copied into a fixed-size allocator briefly
when calling JS plugins.

This dramatically reduces memory usage from n_files × 6 GiB to n_files ×
dynamic + thread_count × 6 GiB.

---------

Co-authored-by: Cameron Clark &lt;cameron.clark@hey.com&gt;
diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs
@@ -352,21 +352,6 @@ impl CliRunner {
             .with_report_unused_directives(report_unused_directives);
 
         let number_of_files = files_to_lint.len();
-
-        // Due to the architecture of the import plugin and JS plugins,
-        // linting a large number of files with both enabled can cause resource exhaustion.
-        // See: https://github.com/oxc-project/oxc/issues/15863
-        if number_of_files > 10_000 && use_cross_module && has_external_linter {
-            print_and_flush_stdout(
-                stdout,
-                &format!(
-                    "Failed to run oxlint.\n{}\n",
-                    render_report(&handler, &OxcDiagnostic::error(format!("Linting {number_of_files} files with both import plugin and JS plugins enabled can cause resource exhaustion.")).with_help("See https://github.com/oxc-project/oxc/issues/15863 for more details."))
-                ),
-            );
-            return CliRunResult::TooManyFilesWithImportAndJsPlugins;
-        }
-
         let tsconfig = basic_options.tsconfig;
         if let Some(path) = tsconfig.as_ref() {
             if path.is_file() {
@@ -404,10 +389,16 @@ impl CliRunner {
             }
         };
 
-        // Configure the file system for external linter if needed
+        // Configure the file system for external linter if needed.
+        // When using the copy-to-fixed-allocator approach (cross-module + JS plugins),
+        // we use `OsFileSystem` instead of `RawTransferFileSystem`, because we use standard allocators for parsing.
         let file_system = if has_external_linter {
             #[cfg(all(feature = "napi", target_pointer_width = "64", target_endian = "little"))]
-            {
+            if use_cross_module {
+                // Use standard file system - source text will be copied to fixed-size allocator later
+                None
+            } else {
+                // Use raw transfer file system - source text goes directly to fixed-size allocator
                 Some(
                     &crate::js_plugins::RawTransferFileSystem
                         as &(dyn oxc_linter::RuntimeFileSystem + Sync + Send),
diff --git a/apps/oxlint/src/result.rs b/apps/oxlint/src/result.rs
@@ -17,7 +17,6 @@ pub enum CliRunResult {
     ConfigFileInitFailed,
     ConfigFileInitSucceeded,
     TsGoLintError,
-    TooManyFilesWithImportAndJsPlugins,
 }
 
 impl Termination for CliRunResult {
@@ -38,8 +37,7 @@ impl Termination for CliRunResult {
             | Self::InvalidOptionSeverityWithoutFilter
             | Self::InvalidOptionSeverityWithoutPluginName
             | Self::InvalidOptionSeverityWithoutRuleName
-            | Self::TsGoLintError
-            | Self::TooManyFilesWithImportAndJsPlugins => ExitCode::FAILURE,
+            | Self::TsGoLintError => ExitCode::FAILURE,
         }
     }
 }
diff --git a/crates/oxc_allocator/src/pool/fixed_size.rs b/crates/oxc_allocator/src/pool/fixed_size.rs
@@ -134,10 +134,6 @@ impl FixedSizeAllocatorPool {
                 return None;
             }
 
-            // Protect against IDs wrapping around.
-            // TODO: Does this work? Do we need it anyway?
-            assert!(id < u32::MAX, "Created too many allocators");
-
             // Try to claim this ID. If another thread got there first, retry with new ID.
             if self
                 .next_id
diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs
@@ -7,7 +7,7 @@ use std::{
     rc::Rc,
 };
 
-use oxc_allocator::Allocator;
+use oxc_allocator::{Allocator, AllocatorPool, CloneIn};
 use oxc_ast::{ast::Program, ast_kind::AST_TYPE_MAX};
 use oxc_ast_macros::ast;
 use oxc_ast_visit::utf8_to_utf16::Utf8ToUtf16;
@@ -148,18 +148,24 @@ impl Linter {
         context_sub_hosts: Vec<ContextSubHost<'a>>,
         allocator: &'a Allocator,
     ) -> Vec<Message> {
-        self.run_with_disable_directives(path, context_sub_hosts, allocator).0
+        self.run_with_disable_directives(path, context_sub_hosts, allocator, None).0
     }
 
     /// Same as `run` but also returns the disable directives for the file
     ///
+    /// # Parameters
+    /// - `js_allocator_pool`: Optional pool of fixed-size allocators for copying AST before JS transfer.
+    ///   When `Some`, the AST will be copied into a fixed-size allocator before passing to JS plugins,
+    ///   allowing the main allocator to be a standard (non-fixed-size) allocator.
+    ///
     /// # Panics
     /// Panics in debug mode if running with and without optimizations produces different diagnostic counts.
     pub fn run_with_disable_directives<'a>(
         &self,
         path: &Path,
         context_sub_hosts: Vec<ContextSubHost<'a>>,
         allocator: &'a Allocator,
+        js_allocator_pool: Option<&AllocatorPool>,
     ) -> (Vec<Message>, Option<DisableDirectives>) {
         let ResolvedLinterState { rules, config, external_rules } = self.config.resolve(path);
 
@@ -350,7 +356,13 @@ impl Linter {
             // can mutably access `ctx_host` via `Rc::get_mut` without panicking due to multiple references.
             drop(rules);
 
-            self.run_external_rules(&external_rules, path, &mut ctx_host, allocator);
+            self.run_external_rules(
+                &external_rules,
+                path,
+                &mut ctx_host,
+                allocator,
+                js_allocator_pool,
+            );
 
             // Report unused directives is now handled differently with type-aware linting
 
@@ -382,65 +394,157 @@ impl Linter {
         (diagnostics, disable_directives)
     }
 
+    #[cfg(all(target_pointer_width = "64", target_endian = "little"))]
     fn run_external_rules<'a>(
         &self,
         external_rules: &[(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)],
         path: &Path,
         ctx_host: &mut Rc<ContextHost<'a>>,
         allocator: &'a Allocator,
+        js_allocator_pool: Option<&AllocatorPool>,
     ) {
         if external_rules.is_empty() {
             return;
         }
 
-        // `external_linter` always exists when `external_rules` is not empty
-        let external_linter = self.external_linter.as_ref().unwrap();
+        // If `js_allocator_pool` is provided, use clone-into-fixed-allocator approach
+        if let Some(js_pool) = js_allocator_pool {
+            self.clone_into_fixed_size_allocator_and_run_external_rules(
+                external_rules,
+                path,
+                ctx_host,
+                js_pool,
+            );
+            return;
+        }
+
+        // Extract `Semantic` from `ContextHost`, and get a mutable reference to `Program`.
+        //
+        // It's not possible to obtain a `&mut Program` while `Semantic` exists, because `Semantic`
+        // contains `AstNodes`, which contains `AstKind`s for every AST nodes, each of which contains
+        // an immutable `&` ref to an AST node.
+        // Obtaining a `&mut Program` while `Semantic` exists would be illegal aliasing.
+        //
+        // So instead we get a pointer to `Program`.
+        // The pointer is obtained initially from `&Program` in `Semantic`, but that pointer
+        // has no provenance for mutation, so can't be converted to `&mut Program`.
+        // So create a new pointer to `Program` which inherits `data_end_ptr`'s provenance,
+        // which does allow mutation.
+        //
+        // We then drop `Semantic`, after which no references to any AST nodes remain.
+        // We can then safety convert the pointer to `&mut Program`.
+        //
+        // `Program` was created in `allocator`, and that allocator is a `FixedSizeAllocator`,
+        // so only has 1 chunk. So `data_end_ptr` and `Program` are within the same allocation.
+        // All callers of `Linter::run` obtain `allocator` and `Semantic` from `ModuleContent`,
+        // which ensure they are in same allocation.
+        // However, we have no static guarantee of this, so strictly speaking it's unsound.
+        // TODO: It would be better to avoid the need for a `&mut Program` here, and so avoid this
+        // sketchy behavior.
+        let ctx_host = Rc::get_mut(ctx_host).unwrap();
+        let semantic = mem::take(ctx_host.semantic_mut());
+        let program_addr = NonNull::from(semantic.nodes().program()).addr();
+        let mut program_ptr =
+            allocator.data_end_ptr().cast::<Program<'a>>().with_addr(program_addr);
+        drop(semantic);
+        // SAFETY: Now that we've dropped `Semantic`, no references to any AST nodes remain,
+        // so can get a mutable reference to `Program` without aliasing violations.
+        let program = unsafe { program_ptr.as_mut() };
+
+        self.convert_and_call_external_linter(external_rules, path, ctx_host, allocator, program);
+    }
+
+    #[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
+    fn run_external_rules<'a>(
+        &self,
+        _external_rules: &[(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)],
+        _path: &Path,
+        _ctx_host: &mut Rc<ContextHost<'a>>,
+        _allocator: &'a Allocator,
+        _js_allocator_pool: Option<&AllocatorPool>,
+    ) {
+        // External rules (JS plugins) are not supported on non-64-bit or big-endian platforms
+    }
 
-        let (program_offset, source_text, span_converter) = {
-            // Extract `Semantic` from `ContextHost`, and get a mutable reference to `Program`.
-            //
-            // It's not possible to obtain a `&mut Program` while `Semantic` exists, because `Semantic`
-            // contains `AstNodes`, which contains `AstKind`s for every AST nodes, each of which contains
-            // an immutable `&` ref to an AST node.
-            // Obtaining a `&mut Program` while `Semantic` exists would be illegal aliasing.
-            //
-            // So instead we get a pointer to `Program`.
-            // The pointer is obtained initially from `&Program` in `Semantic`, but that pointer
-            // has no provenance for mutation, so can't be converted to `&mut Program`.
-            // So create a new pointer to `Program` which inherits `data_end_ptr`'s provenance,
-            // which does allow mutation.
-            //
-            // We then drop `Semantic`, after which no references to any AST nodes remain.
-            // We can then safety convert the pointer to `&mut Program`.
-            //
-            // `Program` was created in `allocator`, and that allocator is a `FixedSizeAllocator`,
-            // so only has 1 chunk. So `data_end_ptr` and `Program` are within the same allocation.
-            // All callers of `Linter::run` obtain `allocator` and `Semantic` from `ModuleContent`,
-            // which ensure they are in same allocation.
-            // However, we have no static guarantee of this, so strictly speaking it's unsound.
-            // TODO: It would be better to avoid the need for a `&mut Program` here, and so avoid this
-            // sketchy behavior.
-            let ctx_host = Rc::get_mut(ctx_host).unwrap();
-            let semantic = mem::take(ctx_host.semantic_mut());
-            let program_addr = NonNull::from(semantic.nodes().program()).addr();
-            let mut program_ptr =
-                allocator.data_end_ptr().cast::<Program<'a>>().with_addr(program_addr);
-            drop(semantic);
-            // SAFETY: Now that we've dropped `Semantic`, no references to any AST nodes remain,
-            // so can get a mutable reference to `Program` without aliasing violations.
-            let program = unsafe { program_ptr.as_mut() };
-
-            // Convert spans to UTF-16
-            let span_converter = Utf8ToUtf16::new(program.source_text);
-            span_converter.convert_program(program);
-            span_converter.convert_comments(&mut program.comments);
-
-            // Get offset of `Program` within buffer (bottom 32 bits of pointer)
-            let program_offset = ptr::from_ref(program) as u32;
-
-            (program_offset, program.source_text, span_converter)
+    /// Clone AST into a fixed-size allocator and run external rules.
+    ///
+    /// This copies the AST and source text from the standard allocator into a fixed-size
+    /// allocator before passing to JS plugins. This allows using standard allocators for
+    /// parsing/linting while still supporting JS plugin raw transfer.
+    #[cfg(all(target_pointer_width = "64", target_endian = "little"))]
+    fn clone_into_fixed_size_allocator_and_run_external_rules(
+        &self,
+        external_rules: &[(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)],
+        path: &Path,
+        ctx_host: &mut Rc<ContextHost<'_>>,
+        js_allocator_pool: &AllocatorPool,
+    ) {
+        let js_allocator = js_allocator_pool.get();
+
+        // Get the original source text and program from semantic
+        let ctx_host_ref = Rc::get_mut(ctx_host).unwrap();
+        let semantic = mem::take(ctx_host_ref.semantic_mut());
+        let original_program = semantic.nodes().program();
+        let original_source_text = original_program.source_text;
+
+        // Copy source text to the START of the fixed-size allocator.
+        // This is critical - the JS deserializer expects source text at offset 0.
+        // SAFETY: `js_allocator` is from a fixed-size allocator pool, which wraps the allocator
+        // in a custom `Drop` that doesn't actually drop it (it returns it to the pool), so the
+        // memory remains valid. This matches the safety requirements of `alloc_bytes_start`.
+        let new_source_text: &str = unsafe {
+            let bytes = original_source_text.as_bytes();
+            let ptr = js_allocator.alloc_bytes_start(bytes.len());
+            ptr::copy_nonoverlapping(bytes.as_ptr(), ptr.as_ptr(), bytes.len());
+            std::str::from_utf8_unchecked(std::slice::from_raw_parts(ptr.as_ptr(), bytes.len()))
         };
 
+        // Clone `Program` into fixed-size allocator.
+        // We need to allocate the `Program` struct ITSELF in the allocator, not just its contents.
+        // `clone_in` returns a value on the stack, but we need it in the allocator for raw transfer.
+        let program: &mut Program<'_> = {
+            let mut program = original_program.clone_in(&js_allocator);
+            program.source_text = new_source_text;
+            js_allocator.alloc(program)
+        };
+
+        // Drop semantic now that we've copied what we need
+        drop(semantic);
+
+        self.convert_and_call_external_linter(
+            external_rules,
+            path,
+            ctx_host_ref,
+            &js_allocator,
+            program,
+        );
+
+        // The `AllocatorGuard` (`js_allocator`) is dropped here, returning the allocator to the pool.
+        // This ensures that we never have too many allocators in play at once, avoiding OOM.
+    }
+
+    /// Convert spans to UTF-16, write metadata, call external linter, and process diagnostics.
+    ///
+    /// This is the common code path shared by both `run_external_rules` and
+    /// `clone_into_fixed_size_allocator_and_run_external_rules`.
+    fn convert_and_call_external_linter(
+        &self,
+        external_rules: &[(ExternalRuleId, ExternalOptionsId, AllowWarnDeny)],
+        path: &Path,
+        ctx_host: &ContextHost<'_>,
+        allocator: &Allocator,
+        program: &mut Program<'_>,
+    ) {
+        let source_text = program.source_text;
+
+        // Convert spans to UTF-16
+        let span_converter = Utf8ToUtf16::new(source_text);
+        span_converter.convert_program(program);
+        span_converter.convert_comments(&mut program.comments);
+
+        // Get offset of `Program` within buffer (bottom 32 bits of pointer)
+        let program_offset = ptr::from_ref(program) as u32;
+
         // Write offset of `Program` in metadata at end of buffer
         let metadata = RawTransferMetadata::new(program_offset);
         let metadata_ptr = allocator.end_ptr().cast::<RawTransferMetadata>();
@@ -470,6 +574,9 @@ impl Linter {
             "{}".to_string()
         });
 
+        // `external_linter` always exists when `external_rules` is not empty
+        let external_linter = self.external_linter.as_ref().unwrap();
+
         // Pass AST and rule IDs + options IDs to JS
         let result = (external_linter.lint_file)(
             path.to_owned(),
diff --git a/crates/oxc_linter/src/service/runtime.rs b/crates/oxc_linter/src/service/runtime.rs

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,6 @@ pub enum CliRunResult {`
`17`	`17`	`ConfigFileInitFailed,`
`18`	`18`	`ConfigFileInitSucceeded,`
`19`	`19`	`TsGoLintError,`
`20`		`- TooManyFilesWithImportAndJsPlugins,`
`21`	`20`	`}`
`22`	`21`
`23`	`22`	`impl Termination for CliRunResult {`
`@@ -38,8 +37,7 @@ impl Termination for CliRunResult {`
`38`	`37`	`\| Self::InvalidOptionSeverityWithoutFilter`
`39`	`38`	`\| Self::InvalidOptionSeverityWithoutPluginName`
`40`	`39`	`\| Self::InvalidOptionSeverityWithoutRuleName`
`41`		`- \| Self::TsGoLintError`
`42`		`- \| Self::TooManyFilesWithImportAndJsPlugins => ExitCode::FAILURE,`
	`40`	`+ \| Self::TsGoLintError => ExitCode::FAILURE,`
`43`	`41`	`}`
`44`	`42`	`}`
`45`	`43`	`}`