fs_events(darwin): fix broken double-checked locking on fsevents_default_loop

src/runtime/node/fs_events.zig

@@ -114,7 +114,8 @@ pub const CoreFoundation = struct {
     RunLoopDefaultMode: *CFStringRef,
 
     pub fn get() CoreFoundation {
-        if (fsevents_cf) |cf| return cf;
+        // No unlocked fast path — see `watch()` below for the reasoning. The
+        // mutex is uncontended after initialization.
         fsevents_mutex.lock();
         defer fsevents_mutex.unlock();
         if (fsevents_cf) |cf| return cf;
@@ -146,7 +147,8 @@ pub const CoreServices = struct {
     kFSEventStreamEventIdSinceNow: FSEventStreamEventId = 18446744073709551615,
 
     pub fn get() CoreServices {
-        if (fsevents_cs) |cs| return cs;
+        // No unlocked fast path — see `watch()` below for the reasoning. The
+        // mutex is uncontended after initialization.
         fsevents_mutex.lock();
         defer fsevents_mutex.unlock();
         if (fsevents_cs) |cs| return cs;
@@ -622,26 +624,38 @@ pub const FSEventsWatcher = struct {
 };
 
 pub fn watch(path: string, recursive: bool, callback: FSEventsWatcher.Callback, updateEnd: FSEventsWatcher.UpdateEndCallback, ctx: ?*anyopaque) !*FSEventsWatcher {
-    if (fsevents_default_loop) |loop| {
-        return FSEventsWatcher.init(loop, path, recursive, callback, updateEnd, ctx);
-    } else {
+    // No unlocked fast path: `fsevents_default_loop` is a plain global and an
+    // unsynchronized read here would be textbook broken DCLP. `Darwin.addWatch`
+    // (path_watcher.zig) calls this WITHOUT holding `manager.mutex`, so two
+    // Workers can enter concurrently; on ARM64 Worker B could observe the
+    // non-null pointer before Worker A's stores inside `FSEventsLoop.init()`
+    // (`this.* = fs_loop`) are visible, and then `registerWatcher()` would lock
+    // a garbage `loop.mutex` / read a garbage `loop.watchers`. Same pattern
+    // `PathWatcherManager.get()` already fixed. `watch()` runs once per
+    // `fs.watch()` call; the mutex is uncontended after initialization.
+    const loop = loop: {
         fsevents_default_loop_mutex.lock();
         defer fsevents_default_loop_mutex.unlock();
         if (fsevents_default_loop == null) {
             fsevents_default_loop = try FSEventsLoop.init();
         }
-        return FSEventsWatcher.init(fsevents_default_loop.?, path, recursive, callback, updateEnd, ctx);
-    }
+        break :loop fsevents_default_loop.?;
+    };
+    // Release `fsevents_default_loop_mutex` before `registerWatcher()` (which
+    // takes `loop.mutex`) so we never nest the two. `loop` is stable once
+    // published — only `closeAndWait()` at process exit ever clears it.
+    return FSEventsWatcher.init(loop, path, recursive, callback, updateEnd, ctx);
 }
 
 pub fn closeAndWait() void {
     if (!bun.Environment.isMac) {
         return;
     }
 
+    // No unlocked fast path — see `watch()` above.
+    fsevents_default_loop_mutex.lock();
+    defer fsevents_default_loop_mutex.unlock();
     if (fsevents_default_loop) |loop| {
-        fsevents_default_loop_mutex.lock();
-        defer fsevents_default_loop_mutex.unlock();
         loop.deinit();
         fsevents_default_loop = null;
     }

test/js/node/watch/fs.watch.worker-init-race.test.ts

@@ -0,0 +1,97 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, isMacOS, tempDir } from "harness";
+
+// Regression test for broken double-checked locking on `fsevents_default_loop`
+// in src/bun.js/node/fs_events.zig.
+//
+// `FSEvents.watch()` is called from `Darwin.addWatch` (path_watcher.zig)
+// WITHOUT holding `manager.mutex` (it's released first to keep lock order
+// one-way). Two Workers can therefore enter `FSEvents.watch()` concurrently.
+//
+// Before the fix the function read `fsevents_default_loop` with no lock and
+// no acquire fence; only the else-branch took `fsevents_default_loop_mutex`.
+// On ARM64 Worker A's store of the pointer could become visible to Worker B
+// before the stores inside `FSEventsLoop.init()` (`this.* = fs_loop`), so
+// Worker B would call `registerWatcher()` on a partially-visible loop and
+// lock a garbage `loop.mutex` / read a garbage `loop.watchers` BabyList.
+// `CoreFoundation.get()` / `CoreServices.get()` had the identical pattern.
+//
+// This is `path_watcher.zig`'s own `PathWatcherManager.get()` comment applied
+// to `fs_events.zig`: drop the unlocked fast path; the mutex is uncontended
+// after initialization.
+//
+// The race requires (a) the very first `fs.watch()` in the process to happen
+// on two threads at once and (b) store reordering, so it is low-probability
+// even on Apple Silicon. This test spawns a fresh process per iteration so
+// the loop is uninitialized each time, and fires several Workers that all
+// call `fs.watch()` as their first statement on distinct directories (so
+// `PathWatcherManager` dedup doesn't serialize them).
+//
+// macOS-only: the FSEvents code path doesn't exist on other platforms.
+test.skipIf(!isMacOS)(
+  "FSEvents: concurrent first fs.watch() from Workers does not observe a partially-initialized loop",
+  async () => {
+    const WORKERS = 8;
+    const files: Record<string, string> = {};
+    for (let i = 0; i < WORKERS; i++) files[`d${i}/f.txt`] = "x";
+    files["worker.js"] = `
+      const fs = require("fs");
+      const { workerData } = require("worker_threads");
+      // First thing this thread does: hit FSEvents.watch() via Darwin.addWatch
+      // with manager.mutex released. Multiple Workers race here on a fresh
+      // process so fsevents_default_loop starts null.
+      const w = fs.watch(workerData.dir, () => {});
+      w.close();
+    `;
+    files["main.js"] = `
+      const path = require("path");
+      const { Worker } = require("worker_threads");
+      const root = process.argv[2];
+      const N = ${WORKERS};
+      let done = 0;
+      let failed = false;
+      for (let i = 0; i < N; i++) {
+        const w = new Worker(path.join(root, "worker.js"), {
+          workerData: { dir: path.join(root, "d" + i) },
+        });
+        w.on("error", err => {
+          failed = true;
+          console.error("worker error:", err);
+          process.exit(1);
+        });
+        // Track completion via 'exit' (fires exactly once per worker no matter
+        // how it ends) so a worker that dies without posting can't hang us.
+        w.on("exit", code => {
+          if (code !== 0 && !failed) {
+            failed = true;
+            console.error("worker exited with code", code);
+            process.exit(1);
+          }
+          if (++done === N && !failed) {
+            console.log("OK");
+            process.exit(0);
+          }
+        });
+      }
+    `;
+
+    using dir = tempDir("fsevents-worker-init-race", files);
+
+    // Fresh process each iteration so the FSEvents loop global starts null
+    // and the DCLP race window exists every time.
+    for (let i = 0; i < 20; i++) {
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "main.js", String(dir)],
+        env: bunEnv,
+        cwd: String(dir),
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect(stderr).toBe("");
+      expect(stdout.trim()).toBe("OK");
+      expect(exitCode).toBe(0);
+    }
+  },
+  60_000,
+);