Skip to content

Commit ce54672

Browse files
pks-tgitster
authored andcommitted
refs: fix corruption by not correctly syncing packed-refs to disk
At GitLab we have recently received a report where a repository was left with a corrupted `packed-refs` file after the node hard-crashed even though `core.fsync=reference` was set. This is something that in theory should not happen if we correctly did the atomic-rename dance to: 1. Write the data into a temporary file. 2. Synchronize the temporary file to disk. 3. Rename the temporary file into place. So if we crash in the middle of writing the `packed-refs` file we should only ever see either the old or the new state of the file. And while we do the dance when writing the `packed-refs` file, there is indeed one gotcha: we use a `FILE *` stream to write the temporary file, but don't flush it before synchronizing it to disk. As a consequence any data that is still buffered will not get synchronized and a crash of the machine may cause corruption. Fix this bug by flushing the file stream before we fsync. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent bc22d84 commit ce54672

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

refs/packed-backend.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1262,7 +1262,8 @@ static int write_with_updates(struct packed_ref_store *refs,
12621262
goto error;
12631263
}
12641264

1265-
if (fsync_component(FSYNC_COMPONENT_REFERENCE, get_tempfile_fd(refs->tempfile)) ||
1265+
if (fflush(out) ||
1266+
fsync_component(FSYNC_COMPONENT_REFERENCE, get_tempfile_fd(refs->tempfile)) ||
12661267
close_tempfile_gently(refs->tempfile)) {
12671268
strbuf_addf(err, "error closing file %s: %s",
12681269
get_tempfile_path(refs->tempfile),

0 commit comments

Comments
 (0)