Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Writes are not flushed before subsequent read #162

Open
WULCAN opened this issue Mar 15, 2025 · 1 comment
Open

Writes are not flushed before subsequent read #162

WULCAN opened this issue Mar 15, 2025 · 1 comment
Assignees
Labels
bug Something isn't working 🐟 medium webapp Related to the Lyra web app

Comments

@WULCAN
Copy link
Collaborator

WULCAN commented Mar 15, 2025

When writing to files, it is possible that data is not immediately flushed to permanent storage. This allows subsequent read operations to see stale data. [Node 21] adds a 'flush' option to the fs.writeFile family of functions which forces the data to be flushed at the end of a successful write operation.

nodejs/node#50009
https://nodejs.org/en/blog/release/v21.0.0

Lyra does use writeFile in a couple of places and it might be important that subsequent read operations do not see stale data.

When Lyra creates a pull request, it first uses writeFile to update files in a local clone, then it instructs git to add changes to the pull request. If git sees stale data we might miss updates and possibly lose data.

We are not sure if this bug actually affects Lyra. The documentation does not really mention this risk, it only refers to manual pages in which the risk seems to be more about kernel crashes.

https://nodejs.org/docs/latest/api/fs.html#fspromiseswritefilefile-data-options
https://nodejs.org/docs/latest/api/fs.html#filehandlesync
https://man7.org/linux/man-pages/man2/fsync.2.html

Still, it seems safer to trust Node when they say that subsequent read operations can see stale data.

Note that flush was later back ported to Node 20: https://nodejs.org/en/blog/release/v20.10.0#new-flush-option-in-file-system-functions

@WULCAN WULCAN added bug Something isn't working webapp Related to the Lyra web app 🐟 medium labels Mar 15, 2025
@amerharb
Copy link
Collaborator

#163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🐟 medium webapp Related to the Lyra web app
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants