diff --git a/crates/oxc_linter/src/module_graph_visitor.rs b/crates/oxc_linter/src/module_graph_visitor.rs index c10302e8f4..78f350c017 100644 --- a/crates/oxc_linter/src/module_graph_visitor.rs +++ b/crates/oxc_linter/src/module_graph_visitor.rs @@ -1,8 +1,4 @@ -use std::{ - marker::PhantomData, - path::PathBuf, - sync::{Arc, Weak}, -}; +use std::{marker::PhantomData, path::PathBuf, sync::Arc}; use rustc_hash::FxHashSet; @@ -186,26 +182,15 @@ impl ModuleGraphVisitor { enter: &mut EnterMod, leave: &mut LeaveMod, ) -> VisitFoldWhile { - // Sort entries to ensure deterministic iteration order. - // The module graph is populated via parallel insertion (par_drain in runtime.rs), - // which causes non-deterministic insertion order into FxHashMap. - // Different iteration orders can cause cycle detection to find or miss cycles - // depending on which path reaches a node first (due to the `traversed` set). - let mut entries: Vec<_> = module_record - .loaded_modules() - .iter() - .map(|(k, v)| (k.clone(), Weak::clone(v))) - .collect(); - entries.sort_unstable_by(|a, b| a.0.cmp(&b.0)); - - for (key, weak_module_record) in entries { + let entries = module_record.sorted_loaded_modules(); + for (key, weak_module_record) in entries.iter() { if self.depth > self.max_depth { return VisitFoldWhile::Stop(accumulator.into_inner()); } let loaded_module_record = weak_module_record.upgrade().unwrap(); - let pair = (&key, &loaded_module_record); + let pair = (key, &loaded_module_record); if !filter(pair, module_record) { continue; diff --git a/crates/oxc_linter/src/module_record.rs b/crates/oxc_linter/src/module_record.rs index 685a7be023..24079d03cc 100644 --- a/crates/oxc_linter/src/module_record.rs +++ b/crates/oxc_linter/src/module_record.rs @@ -13,6 +13,8 @@ use oxc_span::Span; use oxc_str::CompactStr; pub use oxc_syntax::module_record::RequestedModule; +type SortedLoadedModules = Arc<[(CompactStr, Weak)]>; + /// ESM Module Record /// /// All data inside this data structure are for ESM, no commonjs data is allowed. @@ -51,6 +53,8 @@ pub struct ModuleRecord { /// Use [ModuleRecord::get_loaded_module] to get a `ModuleRecord`. loaded_modules: RwLock>>, + sorted_loaded_modules: RwLock>, + /// `[[ImportEntries]]` /// /// A List of `ImportEntry` records derived from the code of this module @@ -105,6 +109,7 @@ impl fmt::Debug for ModuleRecord { .field("resolved_absolute_path", &self.resolved_absolute_path) .field("requested_modules", &self.requested_modules) .field("loaded_modules", &loaded_modules) + .field("sorted_loaded_modules", &self.sorted_loaded_modules) .field("import_entries", &self.import_entries) .field("local_export_entries", &self.local_export_entries) .field("indirect_export_entries", &self.indirect_export_entries) @@ -518,13 +523,43 @@ impl ModuleRecord { self.loaded_modules.read().unwrap() } + /// Returns a cached snapshot of loaded modules sorted by specifier. + /// + /// Stable ordering makes graph walks deterministic despite parallel insertion into the map. + /// Existing snapshots remain valid after changes through [`Self::write_loaded_modules`]. + /// /// # Panics /// - /// * If the RwLock is poisoned (which only happens if a thread panicked while holding the lock). + /// * If either RwLock is poisoned. + pub(crate) fn sorted_loaded_modules(&self) -> SortedLoadedModules { + if let Some(sorted) = self.sorted_loaded_modules.read().unwrap().as_ref() { + return Arc::clone(sorted); + } + + // Keep the map locked until publication, using the same lock order as the writer. + let loaded_modules = self.loaded_modules(); + let mut sorted = self.sorted_loaded_modules.write().unwrap(); + Arc::clone(sorted.get_or_insert_with(|| { + let mut entries: Vec<_> = loaded_modules + .iter() + .map(|(key, module)| (key.clone(), Weak::clone(module))) + .collect(); + entries.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + entries.into() + })) + } + + /// Invalidates the sorted snapshot before granting write access. + /// + /// # Panics + /// + /// * If either RwLock is poisoned. pub fn write_loaded_modules( &self, ) -> RwLockWriteGuard<'_, FxHashMap>> { - self.loaded_modules.write().unwrap() + let loaded_modules = self.loaded_modules.write().unwrap(); + *self.sorted_loaded_modules.write().unwrap() = None; + loaded_modules } /// Get a loaded module by upgrading the weak reference to an Arc. diff --git a/crates/oxc_linter/src/rules/import/no_cycle.rs b/crates/oxc_linter/src/rules/import/no_cycle.rs index 319160b599..a2faecc039 100644 --- a/crates/oxc_linter/src/rules/import/no_cycle.rs +++ b/crates/oxc_linter/src/rules/import/no_cycle.rs @@ -160,21 +160,17 @@ impl Rule for NoCycle { } let needle = &module_record.resolved_absolute_path; - let mut direct_imports = module_record - .loaded_modules() - .iter() - .map(|(key, weak_module_record)| (key.clone(), weak_module_record.upgrade().unwrap())) - .collect::>(); - direct_imports.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + let direct_imports = module_record.sorted_loaded_modules(); let mut stack = Vec::new(); let mut traversed = FxHashSet::default(); - for (key, loaded_module_record) in direct_imports { - if !self.should_traverse_module(&key, &loaded_module_record, module_record) { + for (key, weak_module_record) in direct_imports.iter() { + let loaded_module_record = weak_module_record.upgrade().unwrap(); + if !self.should_traverse_module(key, &loaded_module_record, module_record) { continue; } - let requested_module = module_record.requested_modules[&key][0]; + let requested_module = module_record.requested_modules[key][0]; let span = requested_module.span; stack.clear(); stack.push((key.clone(), Arc::clone(&loaded_module_record)));