From 2db69de81deea4682579d0b9e6da40b4e9558c05 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 10 Aug 2015 11:47:36 +0200 Subject: lockfile: move documentation to lockfile.h and lockfile.c Rearrange/rewrite it somewhat to fit its new environment. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- lockfile.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) (limited to 'lockfile.c') diff --git a/lockfile.c b/lockfile.c index 5a93bc7bc2..2369eff541 100644 --- a/lockfile.c +++ b/lockfile.c @@ -1,6 +1,59 @@ /* * Copyright (c) 2005, Junio C Hamano */ + +/* + * State diagram and cleanup + * ------------------------- + * + * This module keeps track of all locked files in `lock_file_list` for + * use at cleanup. This list and the `lock_file` objects that comprise + * it must be kept in self-consistent states at all time, because the + * program can be interrupted any time by a signal, in which case the + * signal handler will walk through the list attempting to clean up + * any open lock files. + * + * The possible states of a `lock_file` object are as follows: + * + * - Uninitialized. In this state the object's `on_list` field must be + * zero but the rest of its contents need not be initialized. As + * soon as the object is used in any way, it is irrevocably + * registered in `lock_file_list`, and `on_list` is set. + * + * - Locked, lockfile open (after `hold_lock_file_for_update()`, + * `hold_lock_file_for_append()`, or `reopen_lock_file()`). In this + * state: + * + * - the lockfile exists + * - `active` is set + * - `filename` holds the filename of the lockfile + * - `fd` holds a file descriptor open for writing to the lockfile + * - `fp` holds a pointer to an open `FILE` object if and only if + * `fdopen_lock_file()` has been called on the object + * - `owner` holds the PID of the process that locked the file + * + * - 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()`, `commit_lock_file_to()`, + * `rollback_lock_file()`, a failed attempt to lock, or a failed + * `close_lock_file()`). In this state: + * + * - `active` is unset + * - `filename` is empty (usually, though there are transitory + * states in which this condition doesn't hold). Client code should + * *not* rely on the filename being empty in this state. + * - `fd` is -1 + * - the object is left registered in the `lock_file_list`, and + * `on_list` is set. + * + * A lockfile is owned by the process that created it. The `lock_file` + * has an `owner` field that records the owner's PID. This field is + * used to prevent a forked process from closing a lockfile created by + * its parent. + */ + #include "cache.h" #include "lockfile.h" #include "sigchain.h" -- cgit v1.2.3 From c99a4c2db3053e4fb6a43870f5c747f858b0f58f Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 10 Aug 2015 11:47:38 +0200 Subject: lockfile: add accessors get_lock_file_fd() and get_lock_file_fp() We are about to move those members, so change client code to read them through accessor functions. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- lockfile.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'lockfile.c') diff --git a/lockfile.c b/lockfile.c index 2369eff541..df9c704f51 100644 --- a/lockfile.c +++ b/lockfile.c @@ -364,6 +364,20 @@ FILE *fdopen_lock_file(struct lock_file *lk, const char *mode) return lk->fp; } +int get_lock_file_fd(struct lock_file *lk) +{ + if (!lk->active) + die("BUG: get_lock_file_fd() called for unlocked object"); + return lk->fd; +} + +FILE *get_lock_file_fp(struct lock_file *lk) +{ + if (!lk->active) + die("BUG: get_lock_file_fp() called for unlocked object"); + return lk->fp; +} + char *get_locked_file_path(struct lock_file *lk) { if (!lk->active) -- cgit v1.2.3 From b4fb09e4da53bfc6c720337142af5db3204736d5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 10 Aug 2015 11:47:39 +0200 Subject: lockfile: add accessor get_lock_file_path() Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- lockfile.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'lockfile.c') diff --git a/lockfile.c b/lockfile.c index df9c704f51..5e954baf07 100644 --- a/lockfile.c +++ b/lockfile.c @@ -364,6 +364,13 @@ FILE *fdopen_lock_file(struct lock_file *lk, const char *mode) return lk->fp; } +const char *get_lock_file_path(struct lock_file *lk) +{ + if (!lk->active) + die("BUG: get_lock_file_path() called for unlocked object"); + return lk->filename.buf; +} + int get_lock_file_fd(struct lock_file *lk) { if (!lk->active) -- cgit v1.2.3 From 9c77381d6a495e102b811df954d0fa14e62250ab Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 10 Aug 2015 11:47:40 +0200 Subject: commit_lock_file(): use get_locked_file_path() First beef up the sanity checking in get_locked_file_path() to match that in commit_lock_file(). Then rewrite commit_lock_file() to use get_locked_file_path() for its pathname computation. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- lockfile.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) (limited to 'lockfile.c') diff --git a/lockfile.c b/lockfile.c index 5e954baf07..3904803686 100644 --- a/lockfile.c +++ b/lockfile.c @@ -389,8 +389,10 @@ char *get_locked_file_path(struct lock_file *lk) { if (!lk->active) die("BUG: get_locked_file_path() called for unlocked object"); - if (lk->filename.len <= LOCK_SUFFIX_LEN) + if (lk->filename.len <= LOCK_SUFFIX_LEN || + strcmp(lk->filename.buf + lk->filename.len - LOCK_SUFFIX_LEN, LOCK_SUFFIX)) die("BUG: get_locked_file_path() called for malformed lock object"); + /* remove ".lock": */ return xmemdupz(lk->filename.buf, lk->filename.len - LOCK_SUFFIX_LEN); } @@ -458,22 +460,16 @@ int commit_lock_file_to(struct lock_file *lk, const char *path) int commit_lock_file(struct lock_file *lk) { - static struct strbuf result_file = STRBUF_INIT; - int err; + char *result_path = get_locked_file_path(lk); - if (!lk->active) - die("BUG: attempt to commit unlocked object"); - - if (lk->filename.len <= LOCK_SUFFIX_LEN || - strcmp(lk->filename.buf + lk->filename.len - LOCK_SUFFIX_LEN, LOCK_SUFFIX)) - die("BUG: lockfile filename corrupt"); - - /* remove ".lock": */ - strbuf_add(&result_file, lk->filename.buf, - lk->filename.len - LOCK_SUFFIX_LEN); - err = commit_lock_file_to(lk, result_file.buf); - strbuf_reset(&result_file); - return err; + if (commit_lock_file_to(lk, result_path)) { + int save_errno = errno; + free(result_path); + errno = save_errno; + return -1; + } + free(result_path); + return 0; } void rollback_lock_file(struct lock_file *lk) -- cgit v1.2.3 From 1a9d15db25487bb3fc009a88375cc206a60e0e3b Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 10 Aug 2015 11:47:41 +0200 Subject: tempfile: a new module for handling temporary files A lot of work went into defining the state diagram for lockfiles and ensuring correct, race-resistant cleanup in all circumstances. Most of that infrastructure can be applied directly to *any* temporary file. So extract a new "tempfile" module from the "lockfile" module. Reimplement lockfile on top of tempfile. Subsequent commits will add more users of the new module. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- lockfile.c | 261 ++++--------------------------------------------------------- 1 file changed, 16 insertions(+), 245 deletions(-) (limited to 'lockfile.c') diff --git a/lockfile.c b/lockfile.c index 3904803686..e1d68f77ca 100644 --- a/lockfile.c +++ b/lockfile.c @@ -2,90 +2,8 @@ * Copyright (c) 2005, Junio C Hamano */ -/* - * State diagram and cleanup - * ------------------------- - * - * This module keeps track of all locked files in `lock_file_list` for - * use at cleanup. This list and the `lock_file` objects that comprise - * it must be kept in self-consistent states at all time, because the - * program can be interrupted any time by a signal, in which case the - * signal handler will walk through the list attempting to clean up - * any open lock files. - * - * The possible states of a `lock_file` object are as follows: - * - * - Uninitialized. In this state the object's `on_list` field must be - * zero but the rest of its contents need not be initialized. As - * soon as the object is used in any way, it is irrevocably - * registered in `lock_file_list`, and `on_list` is set. - * - * - Locked, lockfile open (after `hold_lock_file_for_update()`, - * `hold_lock_file_for_append()`, or `reopen_lock_file()`). In this - * state: - * - * - the lockfile exists - * - `active` is set - * - `filename` holds the filename of the lockfile - * - `fd` holds a file descriptor open for writing to the lockfile - * - `fp` holds a pointer to an open `FILE` object if and only if - * `fdopen_lock_file()` has been called on the object - * - `owner` holds the PID of the process that locked the file - * - * - 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()`, `commit_lock_file_to()`, - * `rollback_lock_file()`, a failed attempt to lock, or a failed - * `close_lock_file()`). In this state: - * - * - `active` is unset - * - `filename` is empty (usually, though there are transitory - * states in which this condition doesn't hold). Client code should - * *not* rely on the filename being empty in this state. - * - `fd` is -1 - * - the object is left registered in the `lock_file_list`, and - * `on_list` is set. - * - * A lockfile is owned by the process that created it. The `lock_file` - * has an `owner` field that records the owner's PID. This field is - * used to prevent a forked process from closing a lockfile created by - * its parent. - */ - #include "cache.h" #include "lockfile.h" -#include "sigchain.h" - -static struct lock_file *volatile lock_file_list; - -static void remove_lock_files(int skip_fclose) -{ - pid_t me = getpid(); - - while (lock_file_list) { - if (lock_file_list->owner == me) { - /* fclose() is not safe to call in a signal handler */ - if (skip_fclose) - lock_file_list->fp = NULL; - rollback_lock_file(lock_file_list); - } - lock_file_list = lock_file_list->next; - } -} - -static void remove_lock_files_on_exit(void) -{ - remove_lock_files(0); -} - -static void remove_lock_files_on_signal(int signo) -{ - remove_lock_files(1); - sigchain_pop(signo); - raise(signo); -} /* * path = absolute or relative path name @@ -154,60 +72,17 @@ static void resolve_symlink(struct strbuf *path) /* Make sure errno contains a meaningful value on error */ static int lock_file(struct lock_file *lk, const char *path, int flags) { - size_t pathlen = strlen(path); - - if (!lock_file_list) { - /* One-time initialization */ - sigchain_push_common(remove_lock_files_on_signal); - atexit(remove_lock_files_on_exit); - } + int fd; + struct strbuf filename = STRBUF_INIT; - if (lk->active) - die("BUG: cannot lock_file(\"%s\") using active struct lock_file", - path); - if (!lk->on_list) { - /* Initialize *lk and add it to lock_file_list: */ - lk->fd = -1; - lk->fp = NULL; - lk->active = 0; - lk->owner = 0; - strbuf_init(&lk->filename, pathlen + LOCK_SUFFIX_LEN); - lk->next = lock_file_list; - lock_file_list = lk; - lk->on_list = 1; - } else if (lk->filename.len) { - /* This shouldn't happen, but better safe than sorry. */ - die("BUG: lock_file(\"%s\") called with improperly-reset lock_file object", - path); - } + strbuf_addstr(&filename, path); + if (!(flags & LOCK_NO_DEREF)) + resolve_symlink(&filename); - if (flags & LOCK_NO_DEREF) { - strbuf_add_absolute_path(&lk->filename, path); - } else { - struct strbuf resolved_path = STRBUF_INIT; - - strbuf_add(&resolved_path, path, pathlen); - resolve_symlink(&resolved_path); - strbuf_add_absolute_path(&lk->filename, resolved_path.buf); - strbuf_release(&resolved_path); - } - - strbuf_addstr(&lk->filename, LOCK_SUFFIX); - lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666); - if (lk->fd < 0) { - strbuf_reset(&lk->filename); - return -1; - } - lk->owner = getpid(); - lk->active = 1; - if (adjust_shared_perm(lk->filename.buf)) { - int save_errno = errno; - error("cannot fix permission bits on %s", lk->filename.buf); - rollback_lock_file(lk); - errno = save_errno; - return -1; - } - return lk->fd; + strbuf_addstr(&filename, LOCK_SUFFIX); + fd = create_tempfile(&lk->tempfile, filename.buf); + strbuf_release(&filename); + return fd; } static int sleep_microseconds(long us) @@ -353,109 +228,17 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags) return fd; } -FILE *fdopen_lock_file(struct lock_file *lk, const char *mode) -{ - if (!lk->active) - die("BUG: fdopen_lock_file() called for unlocked object"); - if (lk->fp) - die("BUG: fdopen_lock_file() called twice for file '%s'", lk->filename.buf); - - lk->fp = fdopen(lk->fd, mode); - return lk->fp; -} - -const char *get_lock_file_path(struct lock_file *lk) -{ - if (!lk->active) - die("BUG: get_lock_file_path() called for unlocked object"); - return lk->filename.buf; -} - -int get_lock_file_fd(struct lock_file *lk) -{ - if (!lk->active) - die("BUG: get_lock_file_fd() called for unlocked object"); - return lk->fd; -} - -FILE *get_lock_file_fp(struct lock_file *lk) -{ - if (!lk->active) - die("BUG: get_lock_file_fp() called for unlocked object"); - return lk->fp; -} - char *get_locked_file_path(struct lock_file *lk) { - if (!lk->active) - die("BUG: get_locked_file_path() called for unlocked object"); - if (lk->filename.len <= LOCK_SUFFIX_LEN || - strcmp(lk->filename.buf + lk->filename.len - LOCK_SUFFIX_LEN, LOCK_SUFFIX)) + struct strbuf ret = STRBUF_INIT; + + strbuf_addstr(&ret, get_tempfile_path(&lk->tempfile)); + if (ret.len <= LOCK_SUFFIX_LEN || + strcmp(ret.buf + ret.len - LOCK_SUFFIX_LEN, LOCK_SUFFIX)) die("BUG: get_locked_file_path() called for malformed lock object"); /* remove ".lock": */ - return xmemdupz(lk->filename.buf, lk->filename.len - LOCK_SUFFIX_LEN); -} - -int close_lock_file(struct lock_file *lk) -{ - int fd = lk->fd; - FILE *fp = lk->fp; - int err; - - if (fd < 0) - return 0; - - lk->fd = -1; - if (fp) { - lk->fp = NULL; - - /* - * Note: no short-circuiting here; we want to fclose() - * in any case! - */ - err = ferror(fp) | fclose(fp); - } else { - err = close(fd); - } - - if (err) { - int save_errno = errno; - rollback_lock_file(lk); - errno = save_errno; - return -1; - } - - return 0; -} - -int reopen_lock_file(struct lock_file *lk) -{ - if (0 <= lk->fd) - die(_("BUG: reopen a lockfile that is still open")); - if (!lk->active) - die(_("BUG: reopen a lockfile that has been committed")); - lk->fd = open(lk->filename.buf, O_WRONLY); - return lk->fd; -} - -int commit_lock_file_to(struct lock_file *lk, const char *path) -{ - if (!lk->active) - die("BUG: attempt to commit unlocked object to \"%s\"", path); - - if (close_lock_file(lk)) - return -1; - - if (rename(lk->filename.buf, path)) { - int save_errno = errno; - rollback_lock_file(lk); - errno = save_errno; - return -1; - } - - lk->active = 0; - strbuf_reset(&lk->filename); - return 0; + strbuf_setlen(&ret, ret.len - LOCK_SUFFIX_LEN); + return strbuf_detach(&ret, NULL); } int commit_lock_file(struct lock_file *lk) @@ -471,15 +254,3 @@ int commit_lock_file(struct lock_file *lk) free(result_path); return 0; } - -void rollback_lock_file(struct lock_file *lk) -{ - if (!lk->active) - return; - - if (!close_lock_file(lk)) { - unlink_or_warn(lk->filename.buf); - lk->active = 0; - strbuf_reset(&lk->filename); - } -} -- cgit v1.2.3