fs: add atomic option to writeFile

[webdevelopersrinu]

Refs: #49886

fs.writeFile can leave a half written file if the process dies in the middle.
People do the temp file and rename dance themselves, or install
write-file-atomic.

I added an atomic option that does it inside writeFile. It writes a temp
file next to the target, flushes it, then renames it over. A reader gets the
old data or the new data, never half. If a step fails I delete the temp file
and keep the original. It also keeps the old file's permissions. It works
like the flush option, and skips the writeFileSync utf8 fast path.

I kept the parent directory fsync out of this one. It needs a binding change
since you cannot open a directory on Windows. I can add it in a follow up.

I added tests for sync, callback and promises, and for the failure paths.

[codecov]

Codecov Report

❌ Patch coverage is 98.02632% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.96%. Comparing base (6e7818e) to head (e6dfb9f).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/fs/promises.js	91.89%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #65754    +/-   ##
========================================
  Coverage   89.95%   89.96%            
========================================
  Files         757      757            
  Lines      258053   258231   +178     
  Branches    48934    48965    +31     
========================================
+ Hits       232144   232319   +175     
- Misses      16962    16978    +16     
+ Partials     8947     8934    -13     

Files with missing lines	Coverage Δ
lib/fs.js	97.39% <100.00%> (+0.05%)	⬆️
lib/internal/fs/utils.js	96.13% <100.00%> (+0.09%)	⬆️
lib/internal/fs/promises.js	91.63% <91.89%> (+0.31%)	⬆️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[LiviaMedeiros]

Thanks for the contribution! However, i don't think we should add such option in this form. It would cause more problems and footguns than it solves.

• it may fail due to directory permissions (i.e. when we are allowed to write to /path/to/file but not create new file in /path/to/
• it may leave orphaned temp files if process dies midway
• even if it doesn't, it would still pollute the directory with temp file until it's fully written and flushed
• it may fail if temp file gets locked (e.g. by indexing process or some antimalware scanner on windows)
• if may fail if filename with added suffix happens to be too long
• it will temporarily waste space in disk, having both old and new file on it
• it will change target file's inode (breaking connections if nlink > 1)
• the current implementation will break symlinks as well
• the current implementation does flush implicitly while technically these two are independent options (we don't have to force sync to make operation atomic on FS level)
• the current implementation does not preserve file ownership
• it also won't preserve any additional metadata, xattr, ACL, etc.

I'd rather let users 'dance' around it in userland explicitly. In simplest form it's just one additional LoC per writeFile(), it's much clearer in terms of all implications and caveats, and it's much more configurable (e.g. app-wide directory for temp files, periodic cleanup, gitignore-friendly filenames, etc.)

---

Obviously an atomic option might be still welcome if implemented using FS capabilities (for example, utilizing RWF_ATOMIC on libuv side) that can not be accessed from userland.

[LiviaMedeiros]

This probably belongs to fs.writeFileSync rather than fs.writeSync

[webdevelopersrinu]

Fair points, thanks. You are right about the symlink, I checked it.

Should I close this, or is the RWF_ATOMIC route worth trying?

[LiviaMedeiros]

It depends on what's the actual goal here, because referenced #49886 was about fsync rather than atomic operation.

IMHO if the goal is to have atomic option or writeFileAtomic functions in Node.js core, it's not feasible until we have a cross-platform syscalls allowing atomic writes on any filesystem. Then such syscalls can be used in libuv and by Node.js.

If the goal is to have generic/configurable implementation, this should be done by a third party lib (either one of existing npm packages or writing new one).

If the goal is to have atomic writes in some specific practical usecase, this should stay app-specific: sometimes tmpfiles in app-specific directory and renames are acceptable, sometimes it's better to write files and create symlink, sometimes safer to move original file to another path, write and return it back, sometimes the right tool is flock, sometimes app should require CoW-based filesystem to be robust. Sometimes we can guarantee that filesystem is local, sometimes it might be NFS. Sometimes we define atomic as 'safe to mechanically unplug storage', sometimes all we need is thread safety, sometimes it's tmpfs or zram. Unfortunately there's no silver bullet so users have to decide what they really need and what can be sacrificed for that.

RWF_ATOMIC won't make writes logically atomic (it's more like improved flush) and it won't be platform&filesystem agnostic anytime soon, but having it implemented on libuv side with flag similar to COPYFILE_FICLONE is a step forward. Might be better to start with opening an issue in libuv repo first in case there are objections or known limitations.

[webdevelopersrinu]

Got it, that makes sense. Closing this one.

I will open an issue on libuv about RWF_ATOMIC first, like you suggested.