Commit e58281aa by Carlos Martín Nieto Committed by Carlos Martin Nieto

filebuf: make unlocking atomic

When renaming a lock file to its final location, we need to make sure
that it is replaced atomically.

We currently have a workaround for Windows by removing the target file.
This means that the target file, which may be a ref or a packfile, may
cease to exist for a short wile, which shold be avoided.

Implement the workaround only in Windows, by making sure that the file
we want to replace is writable.
parent 90befde4
...@@ -334,8 +334,6 @@ int git_filebuf_commit(git_filebuf *file) ...@@ -334,8 +334,6 @@ int git_filebuf_commit(git_filebuf *file)
file->fd = -1; file->fd = -1;
p_unlink(file->path_original);
if (p_rename(file->path_lock, file->path_original) < 0) { if (p_rename(file->path_lock, file->path_original) < 0) {
giterr_set(GITERR_OS, "Failed to rename lockfile to '%s'", file->path_original); giterr_set(GITERR_OS, "Failed to rename lockfile to '%s'", file->path_original);
goto on_error; goto on_error;
......
...@@ -591,6 +591,31 @@ int p_access(const char* path, mode_t mode) ...@@ -591,6 +591,31 @@ int p_access(const char* path, mode_t mode)
return _waccess(buf, mode); return _waccess(buf, mode);
} }
static int ensure_writable(wchar_t *fpath)
{
DWORD attrs;
attrs = GetFileAttributesW(fpath);
if (attrs == INVALID_FILE_ATTRIBUTES) {
if (GetLastError() == ERROR_FILE_NOT_FOUND)
return 0;
giterr_set(GITERR_OS, "failed to get attributes");
return -1;
}
if (!(attrs & FILE_ATTRIBUTE_READONLY))
return 0;
attrs &= ~FILE_ATTRIBUTE_READONLY;
if (!SetFileAttributesW(fpath, attrs)) {
giterr_set(GITERR_OS, "failed to set attributes");
return -1;
}
return 0;
}
int p_rename(const char *from, const char *to) int p_rename(const char *from, const char *to)
{ {
git_win32_path wfrom; git_win32_path wfrom;
...@@ -602,12 +627,13 @@ int p_rename(const char *from, const char *to) ...@@ -602,12 +627,13 @@ int p_rename(const char *from, const char *to)
if (utf8_to_16_with_errno(wfrom, from) < 0 || if (utf8_to_16_with_errno(wfrom, from) < 0 ||
utf8_to_16_with_errno(wto, to) < 0) utf8_to_16_with_errno(wto, to) < 0)
return -1; return -1;
/* wait up to 50ms if file is locked by another thread or process */ /* wait up to 50ms if file is locked by another thread or process */
rename_tries = 0; rename_tries = 0;
rename_succeeded = 0; rename_succeeded = 0;
while (rename_tries < 10) { while (rename_tries < 10) {
if (MoveFileExW(wfrom, wto, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED) != 0) { if (ensure_writable(wto) == 0 &&
MoveFileExW(wfrom, wto, MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED) != 0) {
rename_succeeded = 1; rename_succeeded = 1;
break; break;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment