filebuf: make unlocking atomiccmn/filebuf-atomic-unlock

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.

2 files changed, 28 insertions, 4 deletions

diff --git a/src/filebuf.c b/src/filebuf.c
index d23bcc11c..25f6e52ef 100644
--- a/src/filebuf.c
+++ b/src/filebuf.c
@@ -334,8 +334,6 @@ int git_filebuf_commit(git_filebuf *file)
 
 	file->fd = -1;
 
-	p_unlink(file->path_original);
-
 	if (p_rename(file->path_lock, file->path_original) < 0) {
 		giterr_set(GITERR_OS, "Failed to rename lockfile to '%s'", file->path_original);
 		goto on_error;
diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c
index 34938431a..fbadb1c9e 100644
--- a/src/win32/posix_w32.c
+++ b/src/win32/posix_w32.c
@@ -591,6 +591,31 @@ int p_access(const char* path, mode_t 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)
 {
 	git_win32_path wfrom;
@@ -602,12 +627,13 @@ int p_rename(const char *from, const char *to)
 	if (utf8_to_16_with_errno(wfrom, from) < 0 ||
 		utf8_to_16_with_errno(wto, to) < 0)
 		return -1;
-	
+
 	/* wait up to 50ms if file is locked by another thread or process */
 	rename_tries = 0;
 	rename_succeeded = 0;
 	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;
 			break;
 		}