diff --git a/apps/oxfmt/src/core/external_formatter.rs b/apps/oxfmt/src/core/external_formatter.rs index d23229760a805..660eec4724537 100644 --- a/apps/oxfmt/src/core/external_formatter.rs +++ b/apps/oxfmt/src/core/external_formatter.rs @@ -1,6 +1,6 @@ use std::{ path::Path, - sync::{Arc, Mutex}, + sync::{Arc, RwLock}, }; use napi::{ @@ -73,21 +73,24 @@ pub type JsSortTailwindClassesCb = ThreadsafeFunction< /// Holds raw ThreadsafeFunctions wrapped in Option for cleanup. /// The TSFNs can be explicitly dropped via `cleanup()` to prevent /// use-after-free during V8 cleanup on Node.js exit. +/// +/// Uses `RwLock` instead of `Mutex` for better performance: the wrapper functions +/// only read from the Option (common path), while only `cleanup()` needs write access. #[derive(Clone)] struct TsfnHandles { - init: Arc>>, - format_embedded: Arc>>, - format_file: Arc>>, - sort_tailwind: Arc>>, + init: Arc>>, + format_embedded: Arc>>, + format_file: Arc>>, + sort_tailwind: Arc>>, } impl TsfnHandles { /// Drop all ThreadsafeFunctions to prevent use-after-free during V8 cleanup. fn cleanup(&self) { - let _ = self.init.lock().unwrap().take(); - let _ = self.format_embedded.lock().unwrap().take(); - let _ = self.format_file.lock().unwrap().take(); - let _ = self.sort_tailwind.lock().unwrap().take(); + let _ = self.init.write().unwrap().take(); + let _ = self.format_embedded.write().unwrap().take(); + let _ = self.format_file.write().unwrap().take(); + let _ = self.sort_tailwind.write().unwrap().take(); } } @@ -145,11 +148,11 @@ impl ExternalFormatter { format_file_cb: JsFormatFileCb, sort_tailwindcss_classes_cb: JsSortTailwindClassesCb, ) -> Self { - // Wrap TSFNs in Arc>> so they can be explicitly dropped - let init_handle = Arc::new(Mutex::new(Some(init_cb))); - let format_embedded_handle = Arc::new(Mutex::new(Some(format_embedded_cb))); - let format_file_handle = Arc::new(Mutex::new(Some(format_file_cb))); - let sort_tailwind_handle = Arc::new(Mutex::new(Some(sort_tailwindcss_classes_cb))); + // Wrap TSFNs in Arc>> so they can be explicitly dropped + let init_handle = Arc::new(RwLock::new(Some(init_cb))); + let format_embedded_handle = Arc::new(RwLock::new(Some(format_embedded_cb))); + let format_file_handle = Arc::new(RwLock::new(Some(format_file_cb))); + let sort_tailwind_handle = Arc::new(RwLock::new(Some(sort_tailwindcss_classes_cb))); // Create handles struct for cleanup let handles = TsfnHandles { @@ -243,10 +246,10 @@ impl ExternalFormatter { // Therefore, just provides a dummy external formatter that consistently returns errors. Self { handles: TsfnHandles { - init: Arc::new(Mutex::new(None)), - format_embedded: Arc::new(Mutex::new(None)), - format_file: Arc::new(Mutex::new(None)), - sort_tailwind: Arc::new(Mutex::new(None)), + init: Arc::new(RwLock::new(None)), + format_embedded: Arc::new(RwLock::new(None)), + format_file: Arc::new(RwLock::new(None)), + sort_tailwind: Arc::new(RwLock::new(None)), }, init: Arc::new(|_| Err("Dummy init called".to_string())), format_embedded: Arc::new(|_, _, _| Err("Dummy format_embedded called".to_string())), @@ -289,11 +292,11 @@ fn language_to_prettier_parser(language: &str) -> Option<&'static str> { /// Wrap JS `initExternalFormatter` callback as a normal Rust function. fn wrap_init_external_formatter( - cb_handle: Arc>>, + cb_handle: Arc>>, ) -> InitExternalFormatterCallback { Arc::new(move |num_threads: usize| { debug_span!("oxfmt::external::init", num_threads = num_threads).in_scope(|| { - let guard = cb_handle.lock().unwrap(); + let guard = cb_handle.read().unwrap(); let Some(cb) = guard.as_ref() else { return Err("JS callback unavailable (environment shutting down)".to_string()); }; @@ -320,11 +323,11 @@ fn wrap_init_external_formatter( /// Wrap JS `formatEmbeddedCode` callback as a normal Rust function. fn wrap_format_embedded( - cb_handle: Arc>>, + cb_handle: Arc>>, ) -> FormatEmbeddedWithConfigCallback { Arc::new(move |options: &Value, parser_name: &str, code: &str| { debug_span!("oxfmt::external::format_embedded", parser = %parser_name).in_scope(|| { - let guard = cb_handle.lock().unwrap(); + let guard = cb_handle.read().unwrap(); let Some(cb) = guard.as_ref() else { return Err("JS callback unavailable (environment shutting down)".to_string()); }; @@ -360,10 +363,12 @@ fn wrap_format_embedded( } /// Wrap JS `formatFile` callback as a normal Rust function. -fn wrap_format_file(cb_handle: Arc>>) -> FormatFileWithConfigCallback { +fn wrap_format_file( + cb_handle: Arc>>, +) -> FormatFileWithConfigCallback { Arc::new(move |options: &Value, parser_name: &str, file_name: &str, code: &str| { debug_span!("oxfmt::external::format_file", parser = %parser_name, file = %file_name).in_scope(|| { - let guard = cb_handle.lock().unwrap(); + let guard = cb_handle.read().unwrap(); let Some(cb) = guard.as_ref() else { return Err("JS callback unavailable (environment shutting down)".to_string()); }; @@ -396,12 +401,12 @@ fn wrap_format_file(cb_handle: Arc>>) -> FormatFile /// Wrap JS `sortTailwindClasses` callback as a normal Rust function. fn wrap_sort_tailwind_classes( - cb_handle: Arc>>, + cb_handle: Arc>>, ) -> TailwindWithConfigCallback { Arc::new(move |filepath: &str, options: &Value, classes: Vec| { debug_span!("oxfmt::external::sort_tailwind", classes_count = classes.len()).in_scope( || { - let guard = cb_handle.lock().unwrap(); + let guard = cb_handle.read().unwrap(); let Some(cb) = guard.as_ref() else { // Return original classes if callback unavailable return classes;