diff --git a/apps/oxfmt/src/core/external_formatter.rs b/apps/oxfmt/src/core/external_formatter.rs index b5480e4470e80..d23229760a805 100644 --- a/apps/oxfmt/src/core/external_formatter.rs +++ b/apps/oxfmt/src/core/external_formatter.rs @@ -1,4 +1,7 @@ -use std::{path::Path, sync::Arc}; +use std::{ + path::Path, + sync::{Arc, Mutex}, +}; use napi::{ Status, @@ -67,6 +70,27 @@ pub type JsSortTailwindClassesCb = ThreadsafeFunction< false, >; +/// 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. +#[derive(Clone)] +struct TsfnHandles { + 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(); + } +} + /// Callback function type for formatting embedded code with config. /// Takes (options, parser_name, code) and returns formatted code or an error. type FormatEmbeddedWithConfigCallback = @@ -90,6 +114,8 @@ type TailwindWithConfigCallback = /// External formatter that wraps a JS callback. #[derive(Clone)] pub struct ExternalFormatter { + /// Handles to raw ThreadsafeFunctions for explicit cleanup + handles: TsfnHandles, pub init: InitExternalFormatterCallback, pub format_embedded: FormatEmbeddedWithConfigCallback, pub format_file: FormatFileWithConfigCallback, @@ -109,17 +135,36 @@ impl std::fmt::Debug for ExternalFormatter { impl ExternalFormatter { /// Create an [`ExternalFormatter`] from JS callbacks. + /// + /// The ThreadsafeFunctions are wrapped in `Arc>>` to allow + /// explicit cleanup via the `cleanup()` method. This prevents use-after-free + /// crashes on Node.js exit when V8 cleans up global handles. pub fn new( init_cb: JsInitExternalFormatterCb, format_embedded_cb: JsFormatEmbeddedCb, format_file_cb: JsFormatFileCb, sort_tailwindcss_classes_cb: JsSortTailwindClassesCb, ) -> Self { - let rust_init = wrap_init_external_formatter(init_cb); - let rust_format_embedded = wrap_format_embedded(format_embedded_cb); - let rust_format_file = wrap_format_file(format_file_cb); - let rust_tailwind = wrap_sort_tailwind_classes(sort_tailwindcss_classes_cb); + // 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))); + + // Create handles struct for cleanup + let handles = TsfnHandles { + init: Arc::clone(&init_handle), + format_embedded: Arc::clone(&format_embedded_handle), + format_file: Arc::clone(&format_file_handle), + sort_tailwind: Arc::clone(&sort_tailwind_handle), + }; + + let rust_init = wrap_init_external_formatter(init_handle); + let rust_format_embedded = wrap_format_embedded(format_embedded_handle); + let rust_format_file = wrap_format_file(format_file_handle); + let rust_tailwind = wrap_sort_tailwind_classes(sort_tailwind_handle); Self { + handles, init: rust_init, format_embedded: rust_format_embedded, format_file: rust_format_file, @@ -127,6 +172,15 @@ impl ExternalFormatter { } } + /// Explicitly drop all ThreadsafeFunctions to prevent use-after-free + /// during V8 cleanup on Node.js exit. + /// + /// This should be called before the function returns to ensure the TSFNs + /// are released while V8 handles are still valid. + pub fn cleanup(&self) { + self.handles.cleanup(); + } + /// Initialize external formatter using the JS callback. pub fn init(&self, num_threads: usize) -> Result, String> { (self.init)(num_threads) @@ -188,6 +242,12 @@ impl ExternalFormatter { // Currently, LSP tests are implemented in Rust, while our external formatter relies on JS. // 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(|_| Err("Dummy init called".to_string())), format_embedded: Arc::new(|_, _, _| Err("Dummy format_embedded called".to_string())), format_file: Arc::new(|_, _, _, _| Err("Dummy format_file called".to_string())), @@ -228,11 +288,17 @@ fn language_to_prettier_parser(language: &str) -> Option<&'static str> { // to temporarily convert the current async task into a blocking context. /// Wrap JS `initExternalFormatter` callback as a normal Rust function. -fn wrap_init_external_formatter(cb: JsInitExternalFormatterCb) -> InitExternalFormatterCallback { +fn wrap_init_external_formatter( + cb_handle: Arc>>, +) -> InitExternalFormatterCallback { Arc::new(move |num_threads: usize| { debug_span!("oxfmt::external::init", num_threads = num_threads).in_scope(|| { - block_on(async { - #[expect(clippy::cast_possible_truncation)] + let guard = cb_handle.lock().unwrap(); + let Some(cb) = guard.as_ref() else { + return Err("JS callback unavailable (environment shutting down)".to_string()); + }; + #[expect(clippy::cast_possible_truncation)] + let result = block_on(async { let status = cb.call_async(FnArgs::from((num_threads as u32,))).await; match status { Ok(promise) => match promise.await { @@ -245,16 +311,24 @@ fn wrap_init_external_formatter(cb: JsInitExternalFormatterCb) -> InitExternalFo Err(format!("Failed to call JS initExternalFormatter callback: {err}")) } } - }) + }); + drop(guard); + result }) }) } /// Wrap JS `formatEmbeddedCode` callback as a normal Rust function. -fn wrap_format_embedded(cb: JsFormatEmbeddedCb) -> FormatEmbeddedWithConfigCallback { +fn wrap_format_embedded( + 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(|| { - block_on(async { + let guard = cb_handle.lock().unwrap(); + let Some(cb) = guard.as_ref() else { + return Err("JS callback unavailable (environment shutting down)".to_string()); + }; + let result = block_on(async { let status = cb .call_async(FnArgs::from(( options.clone(), @@ -278,16 +352,22 @@ fn wrap_format_embedded(cb: JsFormatEmbeddedCb) -> FormatEmbeddedWithConfigCallb "Failed to call JS formatting callback for parser '{parser_name}': {err}" )), } - }) + }); + drop(guard); + result }) }) } /// Wrap JS `formatFile` callback as a normal Rust function. -fn wrap_format_file(cb: JsFormatFileCb) -> 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(|| { - block_on(async { + let guard = cb_handle.lock().unwrap(); + let Some(cb) = guard.as_ref() else { + return Err("JS callback unavailable (environment shutting down)".to_string()); + }; + let result = block_on(async { let status = cb .call_async(FnArgs::from(( options.clone(), @@ -307,17 +387,26 @@ fn wrap_format_file(cb: JsFormatFileCb) -> FormatFileWithConfigCallback { "Failed to call JS formatFile callback for file: '{file_name}', parser: '{parser_name}': {err}" )), } - }) + }); + drop(guard); + result }) }) } /// Wrap JS `sortTailwindClasses` callback as a normal Rust function. -fn wrap_sort_tailwind_classes(cb: JsSortTailwindClassesCb) -> TailwindWithConfigCallback { +fn wrap_sort_tailwind_classes( + 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( || { - block_on(async { + let guard = cb_handle.lock().unwrap(); + let Some(cb) = guard.as_ref() else { + // Return original classes if callback unavailable + return classes; + }; + let result = block_on(async { let args = FnArgs::from((filepath.to_string(), options.clone(), classes.clone())); match cb.call_async(args).await { @@ -333,7 +422,9 @@ fn wrap_sort_tailwind_classes(cb: JsSortTailwindClassesCb) -> TailwindWithConfig classes } } - }) + }); + drop(guard); + result }, ) }) diff --git a/apps/oxfmt/src/main_napi.rs b/apps/oxfmt/src/main_napi.rs index 4f12983ad77c9..fa67ce5853555 100644 --- a/apps/oxfmt/src/main_napi.rs +++ b/apps/oxfmt/src/main_napi.rs @@ -89,16 +89,16 @@ pub async fn run_cli( ); utils::init_tracing(); - match command.mode { + let result = match command.mode { Mode::Lsp => { - run_lsp(external_formatter).await; + run_lsp(external_formatter.clone()).await; ("lsp".to_string(), Some(0)) } Mode::Stdin(_) => { init_miette(); - let result = StdinRunner::new(command, external_formatter).run(); + let result = StdinRunner::new(command, external_formatter.clone()).run(); ("stdin".to_string(), Some(result.exit_code())) } @@ -106,13 +106,20 @@ pub async fn run_cli( init_miette(); init_rayon(command.runtime_options.threads); - let result = - FormatRunner::new(command).with_external_formatter(Some(external_formatter)).run(); + let result = FormatRunner::new(command) + .with_external_formatter(Some(external_formatter.clone())) + .run(); ("cli".to_string(), Some(result.exit_code())) } _ => unreachable!("All other modes must have been handled above match arm"), - } + }; + + // Explicitly drop ThreadsafeFunctions before returning to prevent + // use-after-free during V8 cleanup (Node.js issue with TSFN cleanup timing) + external_formatter.cleanup(); + + result } // --- @@ -164,6 +171,7 @@ pub async fn format( // TODO: Plugins support Ok(_) => {} Err(err) => { + external_formatter.cleanup(); return FormatResult { code: source_text, errors: vec![OxcError::new(format!("Failed to setup external formatter: {err}"))], @@ -173,6 +181,7 @@ pub async fn format( // Determine format strategy from file path let Ok(strategy) = FormatFileStrategy::try_from(PathBuf::from(&filename)) else { + external_formatter.cleanup(); return FormatResult { code: source_text, errors: vec![OxcError::new(format!("Unsupported file type: {filename}"))], @@ -184,6 +193,7 @@ pub async fn format( { Ok(options) => options, Err(err) => { + external_formatter.cleanup(); return FormatResult { code: source_text, errors: vec![OxcError::new(format!("Failed to parse configuration: {err}"))], @@ -192,11 +202,11 @@ pub async fn format( }; // Create formatter and format - let formatter = - SourceFormatter::new(num_of_threads).with_external_formatter(Some(external_formatter)); + let formatter = SourceFormatter::new(num_of_threads) + .with_external_formatter(Some(external_formatter.clone())); // Use `block_in_place()` to avoid nested async runtime access - match tokio::task::block_in_place(|| { + let result = match tokio::task::block_in_place(|| { formatter.format(&strategy, &source_text, resolved_options) }) { CoreFormatResult::Success { code, .. } => FormatResult { code, errors: vec![] }, @@ -204,5 +214,11 @@ pub async fn format( let errors = OxcError::from_diagnostics(&filename, &source_text, diagnostics); FormatResult { code: source_text, errors } } - } + }; + + // Explicitly drop ThreadsafeFunctions before returning to prevent + // use-after-free during V8 cleanup (Node.js issue with TSFN cleanup timing) + external_formatter.cleanup(); + + result }