aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Haggerty <mhagger@alum.mit.edu>2014-10-01 12:28:22 +0200
committerJunio C Hamano <gitster@pobox.com>2014-10-01 13:45:13 -0700
commit8e86c155d2962f5dff83c9d0d88b836bf040c1fa (patch)
tree6b9eef5ab880a3249cdb4cab59a19d19ee3049fc
parent8a1c7533e2ee468a505656abab364780b15004fc (diff)
downloadgit-8e86c155d2962f5dff83c9d0d88b836bf040c1fa.tar.gz
git-8e86c155d2962f5dff83c9d0d88b836bf040c1fa.tar.xz
close_lock_file(): if close fails, roll back
If closing an open lockfile fails, then we cannot be sure of the contents of the lockfile, so there is nothing sensible to do but delete it. This change also insures that the lock_file object is left in a defined state in this error path (namely, unlocked). The only caller that is ultimately affected by this change is try_merge_strategy() -> write_locked_index(), which can call close_lock_file() via various execution paths. This caller uses a static lock_file object which previously could have been reused after a failed close_lock_file() even though it was still in locked state. This change causes the lock_file object to be unlocked on failure, thus fixing this error-handling path. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--Documentation/technical/api-lockfile.txt7
-rw-r--r--lockfile.c28
2 files changed, 22 insertions, 13 deletions
diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt
index 653861075..d3bf940ad 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -162,9 +162,10 @@ close_lock_file::
Take a pointer to the `struct lock_file` initialized with an
earlier call to `hold_lock_file_for_update` or
`hold_lock_file_for_append`, and close the file descriptor.
- Return 0 upon success or a negative value on failure to
- close(2). Usually `commit_lock_file` or `rollback_lock_file`
- should be called after `close_lock_file`.
+ Return 0 upon success. On failure to `close(2)`, return a
+ negative value and rollback the lock file. Usually
+ `commit_lock_file` or `rollback_lock_file` should eventually
+ be called if `close_lock_file` succeeds.
reopen_lock_file::
diff --git a/lockfile.c b/lockfile.c
index c897dd8a9..1d18c67ea 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -37,13 +37,14 @@
* lockfile, and owner holds the PID of the process that locked the
* file.
*
- * - Locked, lockfile closed (after close_lock_file()). Same as the
- * previous state, except that the lockfile is closed and fd is -1.
+ * - Locked, lockfile closed (after successful close_lock_file()).
+ * Same as the previous state, except that the lockfile is closed
+ * and fd is -1.
*
- * - Unlocked (after commit_lock_file(), rollback_lock_file(), or a
- * failed attempt to lock). In this state, filename[0] == '\0' and
- * fd is -1. The object is left registered in the lock_file_list,
- * and on_list is set.
+ * - Unlocked (after commit_lock_file(), rollback_lock_file(), a
+ * failed attempt to lock, or a failed close_lock_file()). In this
+ * state, filename[0] == '\0' and fd is -1. The object is left
+ * registered in the lock_file_list, and on_list is set.
*/
static struct lock_file *lock_file_list;
@@ -284,7 +285,13 @@ int close_lock_file(struct lock_file *lk)
return 0;
lk->fd = -1;
- return close(fd);
+ if (close(fd)) {
+ int save_errno = errno;
+ rollback_lock_file(lk);
+ errno = save_errno;
+ return -1;
+ }
+ return 0;
}
int reopen_lock_file(struct lock_file *lk)
@@ -330,7 +337,8 @@ void rollback_lock_file(struct lock_file *lk)
if (!lk->filename[0])
return;
- close_lock_file(lk);
- unlink_or_warn(lk->filename);
- lk->filename[0] = 0;
+ if (!close_lock_file(lk)) {
+ unlink_or_warn(lk->filename);
+ lk->filename[0] = 0;
+ }
}