Fix path_watcher

[cirospaciari]

What does this PR do?

• Documentation or TypeScript types (it's okay to leave the rest blank in this case)
• Code changes

How did you verify your code works?

Existing tests

[github-actions]

❌ @cirospaciari 2 files with test failures on linux-x64:

• test/bundler/bundler_browser.test.ts
• test/js/node/stubs.test.js

View test output

^{#4c544cd00b34bb5007e7432176e77708d82da889}

[github-actions]

❌ @cirospaciari 2 files with test failures on linux-x64-baseline:

• test/bundler/bundler_browser.test.ts
• test/js/node/stubs.test.js

View test output

^{#4c544cd00b34bb5007e7432176e77708d82da889}

[github-actions]

❌ @cirospaciari 3 files with test failures on bun-darwin-aarch64:

• test/bundler/bundler_browser.test.ts
• test/js/bun/test/test-test.test.ts
• test/js/node/stubs.test.js

View test output

^{#4c544cd00b34bb5007e7432176e77708d82da889}

[github-actions]

❌ @cirospaciari 7 files with test failures on bun-darwin-x64-baseline:

• test/bundler/bundler_browser.test.ts
• test/js/bun/spawn/spawn-streaming-stdin.test.ts
• test/js/bun/spawn/spawn-streaming-stdout.test.ts
• test/js/bun/sqlite/sqlite.test.js
• test/js/node/stubs.test.js
• test/js/third_party/webpack/webpack.test.ts
• test/js/web/timers/setTimeout.test.js

View test output

^{#4c544cd00b34bb5007e7432176e77708d82da889}

[Jarred-Sumner]

This does not fix it? The finalized field is set to true in the same scope as the struct is destroyed, which means all cases the struct is used after finalized is a use after free.

bun/src/bun.js/node/path_watcher.zig

Line 816 in 044a6fa

bun.default_allocator.destroy(this);

[cirospaciari]

finalized

It's only used when pending_directories > 0 (on another thread) finalized just say that the owner is gone, when the last task ends/cancel it will call deinit again (but not emit/flush).

[Jarred-Sumner]

It's not threadsafe though. The finalized boolean would need to be loaded and stored atomically with a @Fence to ensure memory ordering. But even that's not safe because the other thread could be accessing it after deinit

[cirospaciari]

It's not threadsafe though. The finalized boolean would need to be loaded and stored atomically with a @Fence to ensure memory ordering. But even that's not safe because the other thread could be accessing it after deinit

I used mutex to guarantee access to finalized and pending_directories but I found another mistake let me finish it, and you are right about memory ordering

[Jarred-Sumner]

Not all usages are locked by a mutex. If it were, it would hang until after it’s freed and then segfault

[Jarred-Sumner]

Thank you