@matheusbsbs / matheus-bagliano / commits / 8e86c155d2

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>

Michael Haggerty committed Oct 1, 2014 at 12:28 UTC 8e86c155d2962f5dff83c9d0d88b836bf040c1fa

2 files changed +22 -13

Documentation/technical/api-lockfile.txt

+4 -3

@@ -162,9 +162,10 @@ close_lock_file::

162 Take a pointer to the `struct lock_file` initialized with an

163 earlier call to `hold_lock_file_for_update` or

164 `hold_lock_file_for_append`, and close the file descriptor.

165 - Return 0 upon success or a negative value on failure to

166 - close(2). Usually `commit_lock_file` or `rollback_lock_file`

167 - should be called after `close_lock_file`.

165 + Return 0 upon success. On failure to `close(2)`, return a

166 + negative value and rollback the lock file. Usually

167 + `commit_lock_file` or `rollback_lock_file` should eventually

168 + be called if `close_lock_file` succeeds.

169

170 reopen_lock_file::

171

lockfile.c

+18 -10

@@ -37,13 +37,14 @@

37 * lockfile, and owner holds the PID of the process that locked the

38 * file.

39 *

40 - * - Locked, lockfile closed (after close_lock_file()). Same as the

41 - * previous state, except that the lockfile is closed and fd is -1.

40 + * - Locked, lockfile closed (after successful close_lock_file()).

41 + * Same as the previous state, except that the lockfile is closed

42 + * and fd is -1.

43 *

43 - * - Unlocked (after commit_lock_file(), rollback_lock_file(), or a

44 - * failed attempt to lock). In this state, filename[0] == '\0' and

45 - * fd is -1. The object is left registered in the lock_file_list,

46 - * and on_list is set.

44 + * - Unlocked (after commit_lock_file(), rollback_lock_file(), a

45 + * failed attempt to lock, or a failed close_lock_file()). In this

46 + * state, filename[0] == '\0' and fd is -1. The object is left

47 + * registered in the lock_file_list, and on_list is set.

48 */

50 static struct lock_file *lock_file_list;

@@ -284,7 +285,13 @@ int close_lock_file(struct lock_file *lk)

285 return 0;

286

287 lk->fd = -1;

287 - return close(fd);

288 + if (close(fd)) {

289 + int save_errno = errno;

290 + rollback_lock_file(lk);

291 + errno = save_errno;

292 + return -1;

293 + }

294 + return 0;

295 }

296

297 int reopen_lock_file(struct lock_file *lk)

@@ -330,7 +337,8 @@ void rollback_lock_file(struct lock_file *lk)

337 if (!lk->filename[0])

338 return;

339

333 - close_lock_file(lk);

334 - unlink_or_warn(lk->filename);

335 - lk->filename[0] = 0;

340 + if (!close_lock_file(lk)) {

341 + unlink_or_warn(lk->filename);

342 + lk->filename[0] = 0;

343 + }

344 }