From 48f1e35758c3c319ae17f829b1891ebdffee3a3e Mon Sep 17 00:00:00 2001 From: Boshen Date: Fri, 30 Jan 2026 09:48:29 +0000 Subject: [PATCH] fix(oxfmt): prevent ThreadsafeFunction crash on Node.js exit (#18723) Fixes a V8 crash (`Check failed: node->IsInUse()`) that occurs when Node.js exits while ThreadsafeFunction handles are still held in Arc closures. ## Problem When running `pnpm oxfmt --check`, the process crashes on exit with: ``` Fatal error in , line 0 Check failed: node->IsInUse(). ``` The crash happens because: 1. TSFNs are captured in `Arc` closures that may live past V8 cleanup 2. When the `Arc` drops during/after V8 cleanup, the TSFN destructor tries to release an already-freed global handle This is a [known class of issue](https://github.com/napi-rs/napi-rs/issues/1220) with napi-rs ThreadsafeFunction lifecycle management. ## Solution Wrap TSFNs in `Arc>>` and add an explicit `cleanup()` method that drops them before the function returns. This ensures TSFNs are released while V8 handles are still valid. The wrapper functions now check if the TSFN is still available before calling, returning an error if cleanup has already occurred. --- apps/oxfmt/src/core/external_formatter.rs | 127 +++++++++++++++++++--- apps/oxfmt/src/main_napi.rs | 36 ++++-- 2 files changed, 135 insertions(+), 28 deletions(-) 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 }