chore: shorten SAFETY comments (#5963)

crates/node_binding/src/lib.rs

@@ -145,10 +145,9 @@ impl Rspack {
   /// Run the given function with the compiler.
   ///
   /// ## Safety
-  /// 1. The caller must ensure that the `Compiler` is not moved or dropped during the lifetime of the function.
-  /// 2. `CompilerStateGuard` should only be dropped so soon as each `build` or `rebuild` session is finished.
-  ///    Otherwise, this would lead to potential race condition for `Compiler`, especially when `build` or `rebuild`
-  ///    was called on JS side and its previous `build` or `rebuild` was yet to finish.
+  /// 1. The caller must ensure that the `Compiler` is not moved or dropped during the lifetime of the callback.
+  /// 2. `CompilerStateGuard` should and only be dropped so soon as each `Compiler` is free of use.
+  ///    Accessing `Compiler` beyond the lifetime of `CompilerStateGuard` would lead to potential race condition.
   unsafe fn run<R>(
     &mut self,
     env: Env,
@@ -163,8 +162,9 @@ impl Rspack {
       // SAFETY: The mutable reference to `Compiler` is exclusive. It's guaranteed by the running state guard.
       Ok(unsafe { s.compiler.as_mut().get_unchecked_mut() })
     })?;
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
+    // SAFETY:
+    // 1. `Compiler` is pinned and stored on the heap.
+    // 2. `JsReference` (NAPI internal mechanism) keeps `Compiler` alive until its instance getting garbage collected.
     f(
       unsafe { std::mem::transmute::<&mut Compiler, &'static mut Compiler>(*compiler) },
       _guard,

crates/node_binding/src/plugins/interceptor.rs

@@ -250,10 +250,10 @@ impl AsyncSeries2<Compilation, CompilationParams> for CompilerThisCompilationTap
     compilation: &mut Compilation,
     _: &mut CompilationParams,
   ) -> rspack_error::Result<()> {
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
     self.function.call_with_sync(compilation).await
   }
@@ -270,10 +270,10 @@ impl AsyncSeries2<Compilation, CompilationParams> for CompilerCompilationTap {
     compilation: &mut Compilation,
     _: &mut CompilationParams,
   ) -> rspack_error::Result<()> {
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
     self.function.call_with_sync(compilation).await
   }
@@ -290,10 +290,10 @@ impl AsyncSeries2<Compilation, Vec<MakeParam>> for CompilerMakeTap {
     compilation: &mut Compilation,
     _: &mut Vec<MakeParam>,
   ) -> rspack_error::Result<()> {
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
 
     self.function.call_with_promise(compilation).await
@@ -307,10 +307,10 @@ impl AsyncSeries2<Compilation, Vec<MakeParam>> for CompilerMakeTap {
 #[async_trait]
 impl AsyncSeriesBail<Compilation, bool> for CompilerShouldEmitTap {
   async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<Option<bool>> {
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
 
     self.function.call_with_sync(compilation).await
@@ -324,10 +324,10 @@ impl AsyncSeriesBail<Compilation, bool> for CompilerShouldEmitTap {
 #[async_trait]
 impl AsyncSeries<Compilation> for CompilationProcessAssetsTap {
   async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
 
     self.function.call_with_promise(compilation).await

crates/node_binding/src/plugins/mod.rs

@@ -256,10 +256,10 @@ impl rspack_core::Plugin for JsHooksAdapterPlugin {
     if self.is_hook_disabled(&Hook::OptimizeModules) {
       return Ok(());
     }
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
     self.hooks.optimize_modules.call(compilation).await
   }
@@ -271,10 +271,10 @@ impl rspack_core::Plugin for JsHooksAdapterPlugin {
     if self.is_hook_disabled(&Hook::AfterOptimizeModules) {
       return Ok(());
     }
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
     self.hooks.after_optimize_modules.call(compilation).await
   }
@@ -297,10 +297,10 @@ impl rspack_core::Plugin for JsHooksAdapterPlugin {
       return Ok(());
     }
 
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(args.compilation) };
 
     self.hooks.optimize_chunk_modules.call(compilation).await
@@ -314,10 +314,10 @@ impl rspack_core::Plugin for JsHooksAdapterPlugin {
       return Ok(());
     }
 
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
 
     self.hooks.finish_make.call(compilation).await
@@ -343,10 +343,10 @@ impl rspack_core::Plugin for JsHooksAdapterPlugin {
       return Ok(());
     }
 
-    // SAFETY: `Compiler` will not be moved, as it's stored on the heap.
-    // The pointer to `Compilation` is valid for the lifetime of `Compiler`.
-    // `Compiler` is valid through the lifetime before it's closed by calling `Compiler.close()` or gc-ed.
-    // `JsCompilation` is valid through the entire lifetime of `Compilation`.
+    // SAFETY:
+    // 1. `Compiler` is stored on the heap and pinned in binding crate.
+    // 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
+    // 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
     let compilation = unsafe { JsCompilation::from_compilation(compilation) };
 
     self.hooks.finish_modules.call(compilation).await

crates/rspack_core/src/compiler/hmr.rs

@@ -123,10 +123,6 @@ where
         new_compilation.has_module_import_export_change = false;
       }
 
-      fast_set(&mut self.compilation, new_compilation);
-
-      self.compilation.lazy_visit_modules = changed_files.clone();
-
       let setup_make_params = if is_incremental_rebuild_make {
         vec![
           MakeParam::ModifiedFiles(modified_files),
@@ -135,7 +131,15 @@ where
       } else {
         vec![MakeParam::ForceBuildDeps(Default::default())]
       };
+
+      new_compilation.lazy_visit_modules = changed_files.clone();
+
+      // FOR BINDING SAFETY:
+      // Update `compilation` for each rebuild.
+      // Make sure `thisCompilation` hook was called before any other hooks that leverage `JsCompilation`.
+      fast_set(&mut self.compilation, new_compilation);
       self.compile(setup_make_params).await?;
+
       self.cache.begin_idle();
     }
 

crates/rspack_core/src/compiler/mod.rs

@@ -141,7 +141,11 @@ where
   #[instrument(name = "compile", skip_all)]
   async fn compile(&mut self, mut params: Vec<MakeParam>) -> Result<()> {
     let mut compilation_params = self.new_compilation_params();
-    // Fake this compilation as *currently* rebuilding does not create a new compilation
+    // FOR BINDING SAFETY:
+    // Make sure `thisCompilation` hook was called for each `JsCompilation` update before any access to it.
+    // `JsCompiler` tapped `thisCompilation` to update the `JsCompilation` on the JavaScript side.
+    // Otherwise, trying to access the old native `JsCompilation` would cause undefined behavior
+    // as the previous instance might get dropped.
     self
       .plugin_driver
       .compiler_hooks

crates/rspack_napi/src/threadsafe_function.rs

@@ -142,6 +142,7 @@ impl<T: 'static, R: 'static + FromNapiValue + ValidateNapiValue> ThreadsafeFunct
 impl<T: 'static, R: 'static + FromNapiValue> ThreadsafeFunction<T, Promise<R>> {
   /// Call the JS function.
   /// If `Promise<T>` is returned, it will be awaited and its value `T` will be returned.
+  /// Otherwise, an [napi::Error] is returned.
   pub async fn call_with_promise(&self, value: T) -> Result<R> {
     match self.call_async::<Promise<R>>(value).await {
       Ok(r) => match r.await {