watcher: stop serving the watchlist's stored fd to --hot reload reads

src/bundler/bundle_v2.rs

@@ -4373,14 +4373,22 @@ pub mod bv2_impl {
                             };
 
                             // Failures to watch are intentionally ignored.
-                            let _ = this.bun_watcher_mut().unwrap().add_file::<true>(
-                                fd,
-                                &load.path,
-                                bun_wyhash::hash(load.path.as_ref()) as u32,
-                                bun_watcher::Loader(code.loader as u8),
-                                bun_sys::Fd::INVALID,
-                                None,
-                            );
+                            if !matches!(
+                                this.bun_watcher_mut().unwrap().add_file::<true>(
+                                    fd,
+                                    &load.path,
+                                    bun_wyhash::hash(load.path.as_ref()) as u32,
+                                    bun_watcher::Loader(code.loader as u8),
+                                    bun_sys::Fd::INVALID,
+                                    None,
+                                ),
+                                Ok(bun_watcher::FdOwnership::Watcher)
+                            ) && fd.is_valid()
+                            {
+                                // Opened above just for the watcher; it
+                                // wasn't adopted (already watched or error).
+                                let _ = bun_sys::close(fd);
+                            }
                         }
                     }
                 }

src/jsc/AsyncModule.rs

@@ -6,11 +6,7 @@ use bun_core::{OwnedString, String as BunString, ZigString};
 use bun_install::dependency::Dependency;
 use bun_install::{DependencyID, Resolution};
 use bun_io::KeepAlive;
-use bun_options_types::LoaderExt as _;
-use bun_options_types::schema::api;
 use bun_resolver::fs as Fs;
-use bun_resolver::package_json::PackageJSON;
-use bun_sys::Fd;
 
 use crate::virtual_machine::VirtualMachine;
 use crate::{
@@ -26,10 +22,6 @@ pub struct InitOpts<'a> {
     pub specifier: &'a [u8],
     pub path: Fs::Path<'a>,
     pub promise_ptr: Option<*mut *mut JSInternalPromise>,
-    pub fd: Option<Fd>,
-    pub package_json: Option<&'a PackageJSON>,
-    pub loader: bun_ast::Loader,
-    pub hash: u32,
     pub arena: Box<ArenaAllocator>,
     /// Backs `parse_result`'s small `AstVec`s (inline bump chunk); must stay
     /// alive alongside `arena` until the module finishes loading.
@@ -46,14 +38,9 @@ pub struct AsyncModule {
     pub(crate) string_buf: Box<[u8]>,
     referrer_len: u32,
     specifier_len: u32,
-    // `?*PackageJSON` / `*JSGlobalObject` — both are VM-lifetime
-    // backrefs (BACKREF/JSC_BORROW class in LIFETIMES.tsv). `package_json` is
-    // stored as a raw ptr so `AsyncModule` is `'static`-embeddable in
-    // `Queue`/`VirtualMachine` without a phantom lifetime; `global_this` uses
-    // [`crate::GlobalRef`] which encapsulates the single audited deref.
-    pub(crate) package_json: Option<core::ptr::NonNull<PackageJSON>>,
-    pub(crate) loader: api::Loader,
-    pub(crate) hash: u32, // default = u32::MAX
+    // `*JSGlobalObject` is a VM-lifetime backref (BACKREF/JSC_BORROW class in
+    // LIFETIMES.tsv); [`crate::GlobalRef`] encapsulates the single audited
+    // deref.
     pub global_this: crate::GlobalRef,
     pub(crate) arena: Box<ArenaAllocator>,
     /// See [`InitOpts::ast_alloc_state`].
@@ -255,7 +242,6 @@ unsafe extern "C" {
 use core::sync::atomic::Ordering;
 use std::io::Write as _;
 
-use bun_core::strings;
 use bun_install::package_manager::run_tasks;
 use bun_install::{self as install, LogLevel, PackageID};
 
@@ -654,9 +640,6 @@ impl AsyncModule {
             string_buf,
             referrer_len,
             specifier_len,
-            package_json: opts.package_json.map(core::ptr::NonNull::from),
-            loader: opts.loader.to_api(),
-            hash: opts.hash,
             // .stmt_blocks = stmt_blocks,
             // .expr_blocks = expr_blocks,
             global_this: crate::GlobalRef::new(global_object),
@@ -1228,11 +1211,10 @@ impl AsyncModule {
         self.parse_result = parse_result;
         // `print_with_source_map` consumes `ParseResult` by
         // value (it moves `ast` into `print_ast`). Hoist the post-print
-        // reads (`is_commonjs_module` / `input_fd`) above the move so we
+        // read (`is_commonjs_module`) above the move so we
         // can `mem::take` instead of cloning.
         let is_commonjs_module = self.parse_result.ast.has_commonjs_export_names
             || self.parse_result.ast.exports_kind == bun_ast::ExportsKind::Cjs;
-        let input_fd = self.parse_result.input_fd;
         let arena = *self.parse_result.ast.parts.allocator();
         let parse_result = core::mem::replace(&mut self.parse_result, ParseResult::empty(arena));
 
@@ -1303,6 +1285,13 @@ impl AsyncModule {
             );
         }
 
+        // Note: the original parse already registered this file with the
+        // watcher (`maybe_watch_file` runs before the pending-imports
+        // enqueue), and the descriptor it read from is either owned by the
+        // watchlist or was closed by the transpile frame's fd guard — so
+        // there is nothing to re-register here, and `parse_result.input_fd`
+        // must not be used (the number may have been closed and recycled).
+
         // SAFETY: per-thread VM.
         if unsafe { (*jsc_vm).is_watcher_enabled() } {
             // SAFETY: per-thread VM.
@@ -1315,37 +1304,6 @@ impl AsyncModule {
                 )
             };
 
-            if let Some(fd_) = input_fd {
-                if bun_paths::is_absolute(path.text)
-                    && !strings::contains(path.text, b"node_modules")
-                {
-                    // SAFETY: `bun_watcher` is the `*mut ImportWatcher` set
-                    // when `is_watcher_enabled()`; cast recovers the
-                    // concrete type (matches VirtualMachine.rs:2301).
-                    let watcher = unsafe {
-                        &mut *(*jsc_vm)
-                            .bun_watcher
-                            .cast::<crate::hot_reloader::ImportWatcher>()
-                    };
-                    // `bun_watcher::PackageJSON` is an opaque
-                    // forward-decl of `bun_resolver::PackageJSON`;
-                    // the watcher only stores the pointer, so cast through.
-                    // SAFETY: `package_json` (when set) is a VM-lifetime
-                    // backref — outlives the watcher entry.
-                    let package_json = self
-                        .package_json
-                        .map(|p| unsafe { &*p.as_ptr().cast::<bun_watcher::PackageJSON>() });
-                    let _ = watcher.add_file::<true>(
-                        fd_,
-                        path.text,
-                        self.hash,
-                        bun_ast::Loader::from_api(self.loader),
-                        Fd::INVALID,
-                        package_json,
-                    );
-                }
-            }
-
             resolved_source.is_commonjs_module = is_commonjs_module;
 
             return Ok(resolved_source);

src/jsc/RuntimeTranspilerStore.rs

@@ -710,7 +710,6 @@ impl TranspilerJob {
         // as the `Transpiler<'_>` cast above.
         transpiler.linker.resolver = ptr::addr_of_mut!(transpiler.resolver).cast();
 
-        let mut fd: Option<Fd> = None;
         let mut package_json: Option<&'static bun_watcher::PackageJSON> = None;
         let hash = Watcher::get_hash(path.text);
 
@@ -722,21 +721,10 @@ impl TranspilerJob {
         let import_watcher: Option<bun_ptr::ParentRef<ImportWatcher, bun_ptr::Mut>> =
             unsafe { bun_ptr::ParentRef::from_nullable_mut((*vm).bun_watcher.cast()) };
         if let Some(iw) = import_watcher {
-            // The watchlist *is* mutated cross-thread (the watcher thread's
-            // `flush_evictions` closes fds and `swap_remove`s), so snapshot
-            // under the watcher mutex — see
-            // `ImportWatcher::snapshot_fd_and_package_json` doc for the EBADF
-            // race this closes.
-            (fd, package_json) = iw.snapshot_fd_and_package_json(hash);
-            // On Linux, `addFileByPathSlow` inserts watchlist entries with
-            // `fd = invalid_fd` (only kqueue needs the descriptor). Treat
-            // invalid as "no cached fd" so `readFileWithAllocator` opens the
-            // file instead of calling `seekTo` on a bogus handle. The snapshot
-            // helper already filtered `!is_valid()`; additionally reject
-            // stdio-tagged fds here.
-            if fd.is_some_and(|f| f.stdio_tag().is_some()) {
-                fd = None;
-            }
+            // The file is always (re-)opened by path — never through the
+            // watchlist's stored fd; see `ImportWatcher::snapshot_package_json`
+            // for the EBADF/EISDIR race that reading a stored fd reopens.
+            package_json = iw.snapshot_package_json(hash);
         }
 
         // this should be a cheap lookup because 24 bytes == 8 * 3 so it's read 3 machine words
@@ -769,15 +757,13 @@ impl TranspilerJob {
         // only, so skipping `Drop` is sound.
         let mut fallback_source = core::mem::MaybeUninit::<bun_ast::Source>::uninit();
 
-        // Usually, we want to close the input file automatically.
-        //
-        // If we're re-using the file descriptor from the fs watcher
-        // Do not close it because that will break the kqueue-based watcher
+        // Close the input file automatically unless the watcher adopts the
+        // descriptor after the parse (`add_file` below).
         //
         // Note: stored in a `Cell` so the scopeguard closure can capture
         // `&Cell<bool>` and the post-parse writes are visible to it without
         // raw-pointer laundering (which the unused-assignment lint can't see).
-        let should_close_input_file_fd = Cell::new(fd.is_none());
+        let should_close_input_file_fd = Cell::new(true);
 
         let mut input_file_fd: Fd = Fd::INVALID;
 
@@ -798,7 +784,7 @@ impl TranspilerJob {
             path,
             loader,
             dirname_fd: Fd::INVALID,
-            file_descriptor: fd,
+            file_descriptor: None,
             // SAFETY: `input_file_fd` is a stack local declared above and
             // outlives `parse_options`; `addr_of_mut!` avoids forming an
             // intermediate `&mut` so the close-guard's later borrow stays sound.
@@ -888,19 +874,21 @@ impl TranspilerJob {
                     && bun_paths::is_absolute(path.text)
                     && !strings::contains(path.text, b"node_modules")
                 {
-                    should_close_input_file_fd.set(false);
                     if let Some(iw) = import_watcher {
                         // SAFETY: BACKREF — process-lifetime watcher; no other
                         // `&ImportWatcher` is live here, and `add_file` is
                         // thread-safe via watcher mutex.
-                        let _ = unsafe { iw.assume_mut() }.add_file::<true>(
+                        let added = unsafe { iw.assume_mut() }.add_file::<true>(
                             input_file_fd,
                             path.text,
                             hash,
                             loader,
                             Fd::INVALID,
                             package_json,
                         );
+                        if matches!(added, Ok(bun_watcher::FdOwnership::Watcher)) {
+                            should_close_input_file_fd.set(false);
+                        }
                     }
                 }
             }
@@ -914,19 +902,21 @@ impl TranspilerJob {
                 && bun_paths::is_absolute(path.text)
                 && !strings::contains(path.text, b"node_modules")
             {
-                should_close_input_file_fd.set(false);
                 if let Some(iw) = import_watcher {
                     // SAFETY: BACKREF — process-lifetime watcher; no other
                     // `&ImportWatcher` is live here, and `add_file` is
                     // thread-safe via watcher mutex.
-                    let _ = unsafe { iw.assume_mut() }.add_file::<true>(
+                    let added = unsafe { iw.assume_mut() }.add_file::<true>(
                         input_file_fd,
                         path.text,
                         hash,
                         loader,
                         Fd::INVALID,
                         package_json,
                     );
+                    if matches!(added, Ok(bun_watcher::FdOwnership::Watcher)) {
+                        should_close_input_file_fd.set(false);
+                    }
                 }
             }
         }

src/jsc/hot_reloader.rs

@@ -39,48 +39,33 @@ pub enum ImportWatcher {
 const _: () = assert!(bun_watcher::Loader::File.0 == bun_ast::Loader::File as u8);
 
 impl ImportWatcher {
-    /// Look up the cached fd (and `package_json` column) for `hash` under the
-    /// watcher's mutex, snapshotting both before returning.
+    /// Look up the `package_json` column for `hash` under the watcher's
+    /// mutex.
     ///
-    /// The watcher thread's `flush_evictions` (called from `on_file_update`)
-    /// closes the cached fd in pass 1 and `swap_remove`s the entry in pass 2.
-    /// `on_file_update` orders `flush_evictions` *before* `enqueue` so the JS
-    /// thread cannot observe the closed-fd window for the *same* event, but
-    /// nothing serializes a *subsequent* event's `flush_evictions` against the
-    /// JS thread's previous-event reload that re-added the entry: the JS
-    /// thread can read the cached fd here while the watcher thread is between
-    /// pass 1 (close) and pass 2 (remove), surfacing as `EBADF reading
-    /// "<path>"` in `transpiler.rs:read_file_with_allocator` (hot.test.ts
-    /// "should work with sourcemap generation" on debian-aarch64). The race
-    /// is closed by locking the same mutex `append_file_maybe_lock<true>` and
-    /// `flush_evictions` take.
-    pub fn snapshot_fd_and_package_json(
+    /// Deliberately does NOT hand out the stored fd for re-reading the file.
+    /// The stored fd is owned by the watchlist and the watcher thread's
+    /// `flush_evictions` closes it under the mutex (a directory event for an
+    /// edited file evicts its entry, see `on_file_update`); a transpile that
+    /// snapshotted the number here would read from it *after* releasing the
+    /// mutex, surfacing as `EBADF reading "<path>"` — or `EISDIR` once a
+    /// resolver `openat` recycles the number — in
+    /// `transpiler.rs:read_file_with_allocator`
+    /// (watch-many-dirs.test.ts). A stored fd can also point at the
+    /// pre-rename inode after an atomic save and return stale contents.
+    /// Reloads open the file by path instead, and `Watcher::add_file` adopts
+    /// the fresh descriptor afterwards.
+    pub fn snapshot_package_json(
         &self,
         hash: bun_watcher::HashType,
-    ) -> (
-        Option<bun_sys::Fd>,
-        Option<&'static bun_watcher::PackageJSON>,
-    ) {
+    ) -> Option<&'static bun_watcher::PackageJSON> {
         let w = match self {
             ImportWatcher::Hot(w) | ImportWatcher::Watch(w) => w,
-            ImportWatcher::None => return (None, None),
+            ImportWatcher::None => return None,
         };
         let _guard = w.mutex.lock_guard();
-        let Some(index) = w.index_of(hash) else {
-            return (None, None);
-        };
-        let watcher_fd = w.watchlist.items_fd()[index as usize];
-        let package_json = w
-            .watchlist
-            .items::<"package_json", Option<&'static bun_watcher::PackageJSON>>()[index as usize];
-        (
-            if watcher_fd.is_valid() {
-                Some(watcher_fd)
-            } else {
-                None
-            },
-            package_json,
-        )
+        let index = w.index_of(hash)?;
+        w.watchlist
+            .items::<"package_json", Option<&'static bun_watcher::PackageJSON>>()[index as usize]
     }
 
     #[inline]
@@ -106,7 +91,7 @@ impl ImportWatcher {
         // Note: bun_watcher::PackageJSON is an opaque forward-decl;
         // callers cast from `&bun_resolver::PackageJSON`.
         package_json: Option<&'static bun_watcher::PackageJSON>,
-    ) -> bun_sys::Result<()> {
+    ) -> bun_sys::Result<bun_watcher::FdOwnership> {
         match self {
             ImportWatcher::Hot(watcher) | ImportWatcher::Watch(watcher) => watcher
                 .add_file::<COPY_FILE_PATH>(
@@ -117,7 +102,7 @@ impl ImportWatcher {
                     dir_fd,
                     package_json,
                 ),
-            ImportWatcher::None => Ok(()),
+            ImportWatcher::None => Ok(bun_watcher::FdOwnership::Caller),
         }
     }
 }