StanfordSNR / gg

The Stanford Builder
GNU General Public License v3.0
987 stars 70 forks source link

missing fsync in atomic create #32

Open photoszzt opened 5 years ago

photoszzt commented 5 years ago

I think this code is not atomic. https://github.com/StanfordSNR/gg/blob/febdf22643655bf9a89f7ddc8b5849dc7e8f4d9e/src/util/path.cc#L188-L209 Two fsync is missing. You need to fsync the temp file and fsync the containing directory after the rename. See: https://lwn.net/Articles/457667/

JustinAzoff commented 4 years ago

I don't thing consistency in the face of a kernel panic or power failure is a requirement based on how that code is used. Adding 2 fsyncs to every file creation would just slow builds down.

photoszzt commented 4 years ago

@JustinAzoff The problem is if I don't put these two fsyncs, the program fails. It complaints that the file is missing when the file is created but the directory is not fsync.