diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc index 11efad189e8d72..62e3411a6705d6 100644 --- a/Documentation/config/core.adoc +++ b/Documentation/config/core.adoc @@ -348,6 +348,31 @@ confusion unless you know what you are doing (e.g. you are creating a read-only snapshot of the same index to a location different from the repository's usual working tree). +core.lockfilePid:: + A comma-separated list of components for which Git should create + a PID file alongside the lock file. When a lock acquisition fails + and a PID file exists, Git can provide additional diagnostic + information about the process holding the lock, including whether + it is still running. ++ +This feature is disabled by default. You can enable it for specific +components or use `all` to enable for all components. ++ +* `none` disables PID file creation for all components. +* `index` creates PID files for index lock operations. +* `config` creates PID files for config file lock operations. +* `refs` creates PID files for reference lock operations. +* `commit-graph` creates PID files for commit-graph lock operations. +* `midx` creates PID files for multi-pack-index lock operations. +* `shallow` creates PID files for shallow file lock operations. +* `gc` creates PID files for garbage collection lock operations. +* `other` creates PID files for other miscellaneous lock operations. +* `all` enables PID file creation for all components. ++ +The PID file is named by inserting `-pid` before the `.lock` suffix. +For example, if the lock file is `index.lock`, the PID file will be +`index-pid.lock`. + core.logAllRefUpdates:: Enable the reflog. Updates to a ref is logged to the file "`$GIT_DIR/logs/`", by appending the new and old diff --git a/Documentation/git.adoc b/Documentation/git.adoc index ce099e78b8023e..6fdd509d34b381 100644 --- a/Documentation/git.adoc +++ b/Documentation/git.adoc @@ -1010,6 +1010,16 @@ be in the future). the background which do not want to cause lock contention with other operations on the repository. Defaults to `1`. +`GIT_LOCK_PID_INFO`:: + If this Boolean environment variable is set to `1`, Git will create + a `.lock.pid` file alongside each lock file containing the PID of the + process that created the lock. This information is displayed in error + messages when a lock conflict occurs, making it easier to identify + stale locks or debug locking issues. The PID files are automatically + cleaned up via signal and atexit handlers; however, if a process is + terminated abnormally (e.g., SIGKILL), the file may remain as a stale + indicator. Disabled by default. + `GIT_REDIRECT_STDIN`:: `GIT_REDIRECT_STDOUT`:: `GIT_REDIRECT_STDERR`:: diff --git a/apply.c b/apply.c index a2ceb3fb40d3b5..95e3bafabdb105 100644 --- a/apply.c +++ b/apply.c @@ -4173,7 +4173,8 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list) } } - hold_lock_file_for_update(&lock, state->fake_ancestor, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update(&lock, state->fake_ancestor, LOCK_DIE_ON_ERROR, + LOCKFILE_PID_OTHER); res = write_locked_index(&result, &lock, COMMIT_LOCK); discard_index(&result); @@ -4830,7 +4831,8 @@ static int apply_patch(struct apply_state *state, if (state->index_file) hold_lock_file_for_update(&state->lock_file, state->index_file, - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, + LOCKFILE_PID_INDEX); else repo_hold_locked_index(state->repo, &state->lock_file, LOCK_DIE_ON_ERROR); diff --git a/builtin/commit.c b/builtin/commit.c index 0243f17d53c97c..30db6409a14cd7 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -540,7 +540,7 @@ static const char *prepare_index(const char **argv, const char *prefix, path = repo_git_path(the_repository, "next-index-%"PRIuMAX, (uintmax_t) getpid()); hold_lock_file_for_update(&false_lock, path, - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, LOCKFILE_PID_OTHER); create_base_index(current_head); add_remove_files(&partial); diff --git a/builtin/credential-store.c b/builtin/credential-store.c index b74e06cc93d9cd..e1d1be5cd10e98 100644 --- a/builtin/credential-store.c +++ b/builtin/credential-store.c @@ -67,7 +67,8 @@ static void rewrite_credential_file(const char *fn, struct credential *c, int timeout_ms = 1000; repo_config_get_int(the_repository, "credentialstore.locktimeoutms", &timeout_ms); - if (hold_lock_file_for_update_timeout(&credential_lock, fn, 0, timeout_ms) < 0) + if (hold_lock_file_for_update_timeout(&credential_lock, fn, 0, timeout_ms, + LOCKFILE_PID_CONFIG) < 0) die_errno(_("unable to get credential storage lock in %d ms"), timeout_ms); if (extra) print_line(extra); diff --git a/builtin/difftool.c b/builtin/difftool.c index e4bc1f831696a8..15d0af00312409 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -636,7 +636,8 @@ static int run_dir_diff(struct repository *repo, struct lock_file lock = LOCK_INIT; strbuf_reset(&buf); strbuf_addf(&buf, "%s/wtindex", tmpdir.buf); - if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 || + if (hold_lock_file_for_update(&lock, buf.buf, 0, + LOCKFILE_PID_OTHER) < 0 || write_locked_index(&wtindex, &lock, COMMIT_LOCK)) { ret = error("could not write %s", buf.buf); goto finish; diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 4cd0b079b61687..9715f04d2da90c 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -1734,7 +1734,8 @@ static void dump_marks(void) return; } - if (hold_lock_file_for_update(&mark_lock, export_marks_file, 0) < 0) { + if (hold_lock_file_for_update(&mark_lock, export_marks_file, 0, + LOCKFILE_PID_OTHER) < 0) { failure |= error_errno(_("unable to write marks file %s"), export_marks_file); return; diff --git a/builtin/gc.c b/builtin/gc.c index e9a76243aa633d..6ba56d784d587a 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -749,7 +749,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) pidfile_path = repo_git_path(the_repository, "gc.pid"); fd = hold_lock_file_for_update(&lock, pidfile_path, - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, LOCKFILE_PID_GC); if (!force) { static char locking_host[HOST_NAME_MAX + 1]; static char *scan_fmt; @@ -1017,7 +1017,7 @@ int cmd_gc(int argc, if (daemonized) { char *path = repo_git_path(the_repository, "gc.log"); hold_lock_file_for_update(&log_lock, path, - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, LOCKFILE_PID_GC); dup2(get_lock_file_fd(&log_lock), 2); atexit(process_log_file_at_exit); free(path); @@ -1797,7 +1797,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, struct repository *r = the_repository; char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path); - if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) { + if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF, + LOCKFILE_PID_GC) < 0) { /* * Another maintenance command is running. * @@ -2561,7 +2562,8 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit lock_file_timeout_ms = 150; fd = hold_lock_file_for_update_timeout(&lk, filename, LOCK_DIE_ON_ERROR, - lock_file_timeout_ms); + lock_file_timeout_ms, + LOCKFILE_PID_GC); /* * Does this file already exist? With the intended contents? Is it @@ -3372,7 +3374,8 @@ static int update_background_schedule(const struct maintenance_start_opts *opts, struct lock_file lk; char *lock_path = xstrfmt("%s/schedule", the_repository->objects->sources->path); - if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) { + if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF, + LOCKFILE_PID_GC) < 0) { if (errno == EEXIST) error(_("unable to create '%s.lock': %s.\n\n" "Another scheduled git-maintenance(1) process seems to be running in this\n" diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 15d51e60a86533..566df7d229b354 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -341,7 +341,8 @@ static int write_patterns_and_update(struct repository *repo, if (safe_create_leading_directories(repo, sparse_filename)) die(_("failed to create directory for sparse-checkout file")); - hold_lock_file_for_update(&lk, sparse_filename, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update(&lk, sparse_filename, LOCK_DIE_ON_ERROR, + LOCKFILE_PID_OTHER); result = update_working_directory(repo, pl); if (result) { diff --git a/bundle.c b/bundle.c index 42327f9739cc54..25728278fc0b52 100644 --- a/bundle.c +++ b/bundle.c @@ -520,7 +520,8 @@ int create_bundle(struct repository *r, const char *path, bundle_fd = 1; else bundle_fd = hold_lock_file_for_update(&lock, path, - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, + LOCKFILE_PID_OTHER); if (version == -1) version = min_version; diff --git a/cache-tree.c b/cache-tree.c index 2aba47060e95d4..b370fa94d06bd9 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -730,7 +730,8 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state, struct lock_file lock_file = LOCK_INIT; int ret; - hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR, + LOCKFILE_PID_INDEX); entries = read_index_from(index_state, index_path, repo_get_git_dir(the_repository)); diff --git a/commit-graph.c b/commit-graph.c index 80be2ff2c39842..b18249fb738049 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2075,7 +2075,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) char *lock_name = get_commit_graph_chain_filename(ctx->odb_source); hold_lock_file_for_update_mode(&lk, lock_name, - LOCK_DIE_ON_ERROR, 0444); + LOCK_DIE_ON_ERROR, 0444, + LOCKFILE_PID_COMMIT_GRAPH); free(lock_name); graph_layer = mks_tempfile_m(ctx->graph_name, 0444); @@ -2094,7 +2095,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) get_tempfile_fd(graph_layer), get_tempfile_path(graph_layer)); } else { hold_lock_file_for_update_mode(&lk, ctx->graph_name, - LOCK_DIE_ON_ERROR, 0444); + LOCK_DIE_ON_ERROR, 0444, + LOCKFILE_PID_COMMIT_GRAPH); f = hashfd(ctx->r->hash_algo, get_lock_file_fd(&lk), get_lock_file_path(&lk)); } diff --git a/config.c b/config.c index f1def0dcfbacba..ac354275230a72 100644 --- a/config.c +++ b/config.c @@ -2985,7 +2985,8 @@ int repo_config_set_multivar_in_file_gently(struct repository *r, * The lock serves a purpose in addition to locking: the new * contents of .git/config will be written into it. */ - fd = hold_lock_file_for_update(&lock, config_filename, 0); + fd = hold_lock_file_for_update(&lock, config_filename, 0, + LOCKFILE_PID_CONFIG); if (fd < 0) { error_errno(_("could not lock config file %s"), config_filename); ret = CONFIG_NO_LOCK; @@ -3330,7 +3331,8 @@ static int repo_config_copy_or_rename_section_in_file( if (!config_filename) config_filename = filename_buf = repo_git_path(r, "config"); - out_fd = hold_lock_file_for_update(&lock, config_filename, 0); + out_fd = hold_lock_file_for_update(&lock, config_filename, 0, + LOCKFILE_PID_CONFIG); if (out_fd < 0) { ret = error(_("could not lock config file %s"), config_filename); goto out; diff --git a/environment.c b/environment.c index a770b5921d9546..505d36ab64c13e 100644 --- a/environment.c +++ b/environment.c @@ -21,6 +21,7 @@ #include "gettext.h" #include "git-zlib.h" #include "ident.h" +#include "lockfile.h" #include "mailmap.h" #include "object-name.h" #include "repository.h" @@ -324,6 +325,78 @@ static enum fsync_component parse_fsync_components(const char *var, const char * return (current & ~negative) | positive; } +static const struct lockfile_pid_component_name { + const char *name; + enum lockfile_pid_component component_bits; +} lockfile_pid_component_names[] = { + { "index", LOCKFILE_PID_INDEX }, + { "config", LOCKFILE_PID_CONFIG }, + { "refs", LOCKFILE_PID_REFS }, + { "commit-graph", LOCKFILE_PID_COMMIT_GRAPH }, + { "midx", LOCKFILE_PID_MIDX }, + { "shallow", LOCKFILE_PID_SHALLOW }, + { "gc", LOCKFILE_PID_GC }, + { "other", LOCKFILE_PID_OTHER }, + { "all", LOCKFILE_PID_ALL }, +}; + +static enum lockfile_pid_component parse_lockfile_pid_components(const char *var, + const char *string) +{ + enum lockfile_pid_component current = LOCKFILE_PID_DEFAULT; + enum lockfile_pid_component positive = 0, negative = 0; + + while (string) { + size_t len; + const char *ep; + int negated = 0; + int found = 0; + + string = string + strspn(string, ", \t\n\r"); + ep = strchrnul(string, ','); + len = ep - string; + if (!strcmp(string, "none")) { + current = LOCKFILE_PID_NONE; + goto next_name; + } + + if (*string == '-') { + negated = 1; + string++; + len--; + if (!len) + warning(_("invalid value for variable %s"), var); + } + + if (!len) + break; + + for (size_t i = 0; i < ARRAY_SIZE(lockfile_pid_component_names); ++i) { + const struct lockfile_pid_component_name *n = &lockfile_pid_component_names[i]; + + if (strncmp(n->name, string, len)) + continue; + + found = 1; + if (negated) + negative |= n->component_bits; + else + positive |= n->component_bits; + } + + if (!found) { + char *component = xstrndup(string, len); + warning(_("ignoring unknown core.lockfilePid component '%s'"), component); + free(component); + } + +next_name: + string = ep; + } + + return (current & ~negative) | positive; +} + static int git_default_core_config(const char *var, const char *value, const struct config_context *ctx, void *cb) { @@ -532,6 +605,13 @@ static int git_default_core_config(const char *var, const char *value, return 0; } + if (!strcmp(var, "core.lockfilepid")) { + if (!value) + return config_error_nonbool(var); + lockfile_pid_components = parse_lockfile_pid_components(var, value); + return 0; + } + if (!strcmp(var, "core.createobject")) { if (!value) return config_error_nonbool(var); diff --git a/lockfile.c b/lockfile.c index 1d5ed016828746..3d71bf139fb15f 100644 --- a/lockfile.c +++ b/lockfile.c @@ -6,6 +6,9 @@ #include "abspath.h" #include "gettext.h" #include "lockfile.h" +#include "parse.h" +#include "strbuf.h" +#include "wrapper.h" /* * path = absolute or relative path name @@ -71,19 +74,113 @@ static void resolve_symlink(struct strbuf *path) strbuf_reset(&link); } +/* + * Lock PID file functions - write PID to a foo-pid.lock file alongside + * the lock file for debugging stale locks. The PID file is registered + * as a tempfile so it gets cleaned up by signal/atexit handlers. + * + * Naming: For "foo.lock", the PID file is "foo-pid.lock" (not "foo.lock.pid"). + * This avoids collision with the refs namespace. + */ + +/* Global config variable, initialized from core.lockfilePid */ +enum lockfile_pid_component lockfile_pid_components = LOCKFILE_PID_DEFAULT; + +/* + * Path generation helpers. + * Given base path "foo", generate: + * - lock path: "foo.lock" + * - pid path: "foo-pid.lock" + */ +static void get_lock_path(struct strbuf *out, const char *path) +{ + strbuf_addstr(out, path); + strbuf_addstr(out, LOCK_SUFFIX); +} + +static void get_pid_path(struct strbuf *out, const char *path) +{ + strbuf_addstr(out, path); + strbuf_addstr(out, LOCK_PID_INFIX); + strbuf_addstr(out, LOCK_SUFFIX); +} + +static struct tempfile *create_lock_pid_file(const char *pid_path, int mode, + enum lockfile_pid_component component) +{ + struct strbuf content = STRBUF_INIT; + struct tempfile *pid_tempfile = NULL; + int fd = -1; + + if (!(lockfile_pid_components & component)) + goto out; + + fd = open(pid_path, O_WRONLY | O_CREAT | O_EXCL, mode); + if (fd < 0) + goto out; + + strbuf_addf(&content, "%" PRIuMAX "\n", (uintmax_t)getpid()); + if (write_in_full(fd, content.buf, content.len) < 0) { + warning_errno(_("could not write lock pid file '%s'"), pid_path); + goto out; + } + + close(fd); + fd = -1; + pid_tempfile = register_tempfile(pid_path); + +out: + if (fd >= 0) + close(fd); + strbuf_release(&content); + return pid_tempfile; +} + +static int read_lock_pid(const char *pid_path, uintmax_t *pid_out) +{ + struct strbuf content = STRBUF_INIT; + int ret = -1; + + if (strbuf_read_file(&content, pid_path, LOCK_PID_MAXLEN) <= 0) + goto out; + + { + char *endptr; + *pid_out = strtoumax(content.buf, &endptr, 10); + if (*pid_out > 0 && (*endptr == '\n' || *endptr == '\0')) + ret = 0; + else + warning(_("malformed lock pid file '%s'"), pid_path); + } + +out: + strbuf_release(&content); + return ret; +} + /* Make sure errno contains a meaningful value on error */ static int lock_file(struct lock_file *lk, const char *path, int flags, - int mode) + int mode, enum lockfile_pid_component component) { - struct strbuf filename = STRBUF_INIT; + struct strbuf base_path = STRBUF_INIT; + struct strbuf lock_path = STRBUF_INIT; + struct strbuf pid_path = STRBUF_INIT; - strbuf_addstr(&filename, path); + strbuf_addstr(&base_path, path); if (!(flags & LOCK_NO_DEREF)) - resolve_symlink(&filename); + resolve_symlink(&base_path); + + get_lock_path(&lock_path, base_path.buf); + get_pid_path(&pid_path, base_path.buf); + + lk->tempfile = create_tempfile_mode(lock_path.buf, mode); + if (lk->tempfile) + lk->pid_tempfile = create_lock_pid_file(pid_path.buf, mode, + component); - strbuf_addstr(&filename, LOCK_SUFFIX); - lk->tempfile = create_tempfile_mode(filename.buf, mode); - strbuf_release(&filename); + strbuf_release(&base_path); + strbuf_release(&lock_path); + strbuf_release(&pid_path); return lk->tempfile ? lk->tempfile->fd : -1; } @@ -102,7 +199,8 @@ static int lock_file(struct lock_file *lk, const char *path, int flags, * exactly once. If timeout_ms is -1, try indefinitely. */ static int lock_file_timeout(struct lock_file *lk, const char *path, - int flags, long timeout_ms, int mode) + int flags, long timeout_ms, int mode, + enum lockfile_pid_component component) { int n = 1; int multiplier = 1; @@ -110,7 +208,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, static int random_initialized = 0; if (timeout_ms == 0) - return lock_file(lk, path, flags, mode); + return lock_file(lk, path, flags, mode, component); if (!random_initialized) { srand((unsigned int)getpid()); @@ -124,7 +222,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, long backoff_ms, wait_ms; int fd; - fd = lock_file(lk, path, flags, mode); + fd = lock_file(lk, path, flags, mode, component); if (fd >= 0) return fd; /* success */ @@ -151,16 +249,47 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, void unable_to_lock_message(const char *path, int err, struct strbuf *buf) { if (err == EEXIST) { - strbuf_addf(buf, _("Unable to create '%s.lock': %s.\n\n" - "Another git process seems to be running in this repository, e.g.\n" - "an editor opened by 'git commit'. Please make sure all processes\n" - "are terminated then try again. If it still fails, a git process\n" - "may have crashed in this repository earlier:\n" - "remove the file manually to continue."), - absolute_path(path), strerror(err)); - } else + const char *abs_path = absolute_path(path); + struct strbuf lock_path = STRBUF_INIT; + struct strbuf pid_path = STRBUF_INIT; + uintmax_t pid; + int pid_status = 0; /* 0 = unknown, 1 = running, -1 = stale */ + + get_lock_path(&lock_path, abs_path); + get_pid_path(&pid_path, abs_path); + + strbuf_addf(buf, _("Unable to create '%s': %s.\n\n"), + lock_path.buf, strerror(err)); + + /* + * Try to read PID file unconditionally - it may exist if + * core.lockfilePid was enabled for this component. + */ + if (!read_lock_pid(pid_path.buf, &pid)) { + if (kill((pid_t)pid, 0) == 0) + pid_status = 1; + else + pid_status = -1; + } + + if (pid_status == 1) + strbuf_addf(buf, _("Lock is held by process %" PRIuMAX ". " + "Wait for it to finish, or remove the lock file to continue"), + pid); + else if (pid_status == -1) + strbuf_addf(buf, _("Lock was held by process %" PRIuMAX ", " + "which is no longer running. Remove the stale lock file to continue"), + pid); + else + strbuf_addstr(buf, _("Another git process seems to be running in this repository. " + "Wait for it to finish, or remove the lock file to continue")); + + strbuf_release(&lock_path); + strbuf_release(&pid_path); + } else { strbuf_addf(buf, _("Unable to create '%s.lock': %s"), absolute_path(path), strerror(err)); + } } NORETURN void unable_to_lock_die(const char *path, int err) @@ -174,9 +303,10 @@ NORETURN void unable_to_lock_die(const char *path, int err) /* This should return a meaningful errno on failure */ int hold_lock_file_for_update_timeout_mode(struct lock_file *lk, const char *path, int flags, - long timeout_ms, int mode) + long timeout_ms, int mode, + enum lockfile_pid_component component) { - int fd = lock_file_timeout(lk, path, flags, timeout_ms, mode); + int fd = lock_file_timeout(lk, path, flags, timeout_ms, mode, component); if (fd < 0) { if (flags & LOCK_DIE_ON_ERROR) unable_to_lock_die(path, errno); @@ -207,6 +337,8 @@ int commit_lock_file(struct lock_file *lk) { char *result_path = get_locked_file_path(lk); + delete_tempfile(&lk->pid_tempfile); + if (commit_lock_file_to(lk, result_path)) { int save_errno = errno; free(result_path); @@ -216,3 +348,9 @@ int commit_lock_file(struct lock_file *lk) free(result_path); return 0; } + +int rollback_lock_file(struct lock_file *lk) +{ + delete_tempfile(&lk->pid_tempfile); + return delete_tempfile(&lk->tempfile); +} diff --git a/lockfile.h b/lockfile.h index 1bb99264976d27..7516c08d5487f7 100644 --- a/lockfile.h +++ b/lockfile.h @@ -119,6 +119,7 @@ struct lock_file { struct tempfile *tempfile; + struct tempfile *pid_tempfile; }; #define LOCK_INIT { 0 } @@ -127,6 +128,43 @@ struct lock_file { #define LOCK_SUFFIX ".lock" #define LOCK_SUFFIX_LEN 5 +/* + * PID file naming: for a lock file "foo.lock", the PID file is "foo-pid.lock". + * This avoids collision with refs namespace (unlike "foo.lock.pid"). + */ +#define LOCK_PID_INFIX "-pid" +#define LOCK_PID_INFIX_LEN 4 + +/* Maximum length for PID file content */ +#define LOCK_PID_MAXLEN 32 + +/* + * Per-component lock PID file configuration, following core.fsync pattern. + * Each component can be individually enabled via core.lockfilePid config. + */ +enum lockfile_pid_component { + LOCKFILE_PID_NONE = 0, + LOCKFILE_PID_INDEX = 1 << 0, /* .git/index.lock */ + LOCKFILE_PID_CONFIG = 1 << 1, /* .git/config.lock */ + LOCKFILE_PID_REFS = 1 << 2, /* refs locks */ + LOCKFILE_PID_COMMIT_GRAPH = 1 << 3, /* commit-graph.lock */ + LOCKFILE_PID_MIDX = 1 << 4, /* multi-pack-index.lock */ + LOCKFILE_PID_SHALLOW = 1 << 5, /* shallow file */ + LOCKFILE_PID_GC = 1 << 6, /* gc locks */ + LOCKFILE_PID_OTHER = 1 << 7, /* other locks */ +}; + +#define LOCKFILE_PID_ALL (LOCKFILE_PID_INDEX | LOCKFILE_PID_CONFIG | \ + LOCKFILE_PID_REFS | LOCKFILE_PID_COMMIT_GRAPH | \ + LOCKFILE_PID_MIDX | LOCKFILE_PID_SHALLOW | \ + LOCKFILE_PID_GC | LOCKFILE_PID_OTHER) +#define LOCKFILE_PID_DEFAULT LOCKFILE_PID_NONE + +/* + * Bitmask indicating which components should create PID files. + * Configured via core.lockfilePid. + */ +extern enum lockfile_pid_component lockfile_pid_components; /* * Flags @@ -167,17 +205,23 @@ struct lock_file { * timeout_ms milliseconds. If timeout_ms is 0, try exactly once; if * timeout_ms is -1, retry indefinitely. The flags argument, error * handling, and mode are described above. + * + * The `component` argument specifies which lock PID component this lock + * belongs to, for selective PID file creation via core.lockfilePid config. */ int hold_lock_file_for_update_timeout_mode( - struct lock_file *lk, const char *path, - int flags, long timeout_ms, int mode); + struct lock_file *lk, const char *path, + int flags, long timeout_ms, int mode, + enum lockfile_pid_component component); static inline int hold_lock_file_for_update_timeout( - struct lock_file *lk, const char *path, - int flags, long timeout_ms) + struct lock_file *lk, const char *path, + int flags, long timeout_ms, + enum lockfile_pid_component component) { return hold_lock_file_for_update_timeout_mode(lk, path, flags, - timeout_ms, 0666); + timeout_ms, 0666, + component); } /* @@ -186,17 +230,18 @@ static inline int hold_lock_file_for_update_timeout( * argument and error handling are described above. */ static inline int hold_lock_file_for_update( - struct lock_file *lk, const char *path, - int flags) + struct lock_file *lk, const char *path, + int flags, enum lockfile_pid_component component) { - return hold_lock_file_for_update_timeout(lk, path, flags, 0); + return hold_lock_file_for_update_timeout(lk, path, flags, 0, component); } static inline int hold_lock_file_for_update_mode( - struct lock_file *lk, const char *path, - int flags, int mode) + struct lock_file *lk, const char *path, + int flags, int mode, enum lockfile_pid_component component) { - return hold_lock_file_for_update_timeout_mode(lk, path, flags, 0, mode); + return hold_lock_file_for_update_timeout_mode(lk, path, flags, 0, mode, + component); } /* @@ -319,13 +364,10 @@ static inline int commit_lock_file_to(struct lock_file *lk, const char *path) /* * Roll back `lk`: close the file descriptor and/or file pointer and - * remove the lockfile. It is a NOOP to call `rollback_lock_file()` - * for a `lock_file` object that has already been committed or rolled - * back. No error will be returned in this case. + * remove the lockfile and any associated PID file. It is a NOOP to + * call `rollback_lock_file()` for a `lock_file` object that has already + * been committed or rolled back. No error will be returned in this case. */ -static inline int rollback_lock_file(struct lock_file *lk) -{ - return delete_tempfile(&lk->tempfile); -} +int rollback_lock_file(struct lock_file *lk); #endif /* LOCKFILE_H */ diff --git a/loose.c b/loose.c index 56cf64b648bf80..c0d2f0416f1d33 100644 --- a/loose.c +++ b/loose.c @@ -135,7 +135,8 @@ int repo_write_loose_object_map(struct repository *repo) return 0; repo_common_path_replace(repo, &path, "objects/loose-object-idx"); - fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1); + fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1, + LOCKFILE_PID_OTHER); iter = kh_begin(map); if (write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0) goto errout; @@ -177,7 +178,8 @@ static int write_one_object(struct odb_source *source, struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT; strbuf_addf(&path, "%s/loose-object-idx", source->path); - hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1); + hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1, + LOCKFILE_PID_OTHER); fd = open(path.buf, O_WRONLY | O_CREAT | O_APPEND, 0666); if (fd < 0) diff --git a/midx-write.c b/midx-write.c index 23e61cb0001428..6196dcd7cd0406 100644 --- a/midx-write.c +++ b/midx-write.c @@ -1309,7 +1309,8 @@ static int write_midx_internal(struct odb_source *source, struct strbuf lock_name = STRBUF_INIT; get_midx_chain_filename(source, &lock_name); - hold_lock_file_for_update(&lk, lock_name.buf, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update(&lk, lock_name.buf, LOCK_DIE_ON_ERROR, + LOCKFILE_PID_MIDX); strbuf_release(&lock_name); incr = mks_tempfile_m(midx_name.buf, 0444); @@ -1327,7 +1328,8 @@ static int write_midx_internal(struct odb_source *source, f = hashfd(r->hash_algo, get_tempfile_fd(incr), get_tempfile_path(incr)); } else { - hold_lock_file_for_update(&lk, midx_name.buf, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update(&lk, midx_name.buf, LOCK_DIE_ON_ERROR, + LOCKFILE_PID_MIDX); f = hashfd(r->hash_algo, get_lock_file_fd(&lk), get_lock_file_path(&lk)); } diff --git a/odb.c b/odb.c index 3ec21ef24e16bb..570cea3029e943 100644 --- a/odb.c +++ b/odb.c @@ -292,7 +292,8 @@ void odb_add_to_alternates_file(struct object_database *odb, FILE *in, *out; int found = 0; - hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR, + LOCKFILE_PID_OTHER); out = fdopen_lock_file(&lock, "w"); if (!out) die_errno(_("unable to fdopen alternates lockfile")); diff --git a/refs/files-backend.c b/refs/files-backend.c index 6f6f76a8d86dc4..e296154fe73f3e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -790,7 +790,8 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs, if (hold_lock_file_for_update_timeout( &lock->lk, ref_file.buf, LOCK_NO_DEREF, - get_files_ref_lock_timeout_ms()) < 0) { + get_files_ref_lock_timeout_ms(), + LOCKFILE_PID_REFS) < 0) { int myerr = errno; errno = 0; if (myerr == ENOENT && --attempts_remaining > 0) { @@ -1194,7 +1195,8 @@ static int create_reflock(const char *path, void *cb) return hold_lock_file_for_update_timeout( lk, path, LOCK_NO_DEREF, - get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0; + get_files_ref_lock_timeout_ms(), + LOCKFILE_PID_REFS) < 0 ? -1 : 0; } /* @@ -3536,7 +3538,8 @@ static int files_reflog_expire(struct ref_store *ref_store, * work we need, including cleaning up if the program * exits unexpectedly. */ - if (hold_lock_file_for_update(&reflog_lock, log_file, 0) < 0) { + if (hold_lock_file_for_update(&reflog_lock, log_file, 0, + LOCKFILE_PID_REFS) < 0) { struct strbuf err = STRBUF_INIT; unable_to_lock_message(log_file, errno, &err); error("%s", err.buf); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4ea0c1229946bd..f832c49eae0850 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1230,7 +1230,8 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err) if (hold_lock_file_for_update_timeout( &refs->lock, refs->path, - flags, timeout_value) < 0) { + flags, timeout_value, + LOCKFILE_PID_REFS) < 0) { unable_to_lock_message(refs->path, errno, err); return -1; } diff --git a/reftable/system.c b/reftable/system.c index 725a25844ea179..5462111a04b61d 100644 --- a/reftable/system.c +++ b/reftable/system.c @@ -67,7 +67,7 @@ int flock_acquire(struct reftable_flock *l, const char *target_path, return REFTABLE_OUT_OF_MEMORY_ERROR; err = hold_lock_file_for_update_timeout(lockfile, target_path, LOCK_NO_DEREF, - timeout_ms); + timeout_ms, LOCKFILE_PID_REFS); if (err < 0) { reftable_free(lockfile); if (errno == EEXIST) diff --git a/repository.c b/repository.c index 6aaa7ba00869bf..30dde7076b4bd6 100644 --- a/repository.c +++ b/repository.c @@ -460,5 +460,6 @@ int repo_hold_locked_index(struct repository *repo, { if (!repo->index_file) BUG("the repo hasn't been setup"); - return hold_lock_file_for_update(lf, repo->index_file, flags); + return hold_lock_file_for_update(lf, repo->index_file, flags, + LOCKFILE_PID_INDEX); } diff --git a/rerere.c b/rerere.c index 6ec55964e2ae4a..71f1d5b1762400 100644 --- a/rerere.c +++ b/rerere.c @@ -917,7 +917,8 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags) else fd = hold_lock_file_for_update(&write_lock, git_path_merge_rr(r), - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, + LOCKFILE_PID_OTHER); read_rr(r, merge_rr); return fd; } diff --git a/sequencer.c b/sequencer.c index 5476d39ba9b097..337f82e02c3eba 100644 --- a/sequencer.c +++ b/sequencer.c @@ -566,7 +566,8 @@ static int write_message(const void *buf, size_t len, const char *filename, { struct lock_file msg_file = LOCK_INIT; - int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0); + int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0, + LOCKFILE_PID_OTHER); if (msg_fd < 0) return error_errno(_("could not lock '%s'"), filename); if (write_in_full(msg_fd, buf, len) < 0) { @@ -3605,7 +3606,8 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts, if (is_rebase_i(opts) && !reschedule) next++; - fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); + fd = hold_lock_file_for_update(&todo_lock, todo_path, 0, + LOCKFILE_PID_OTHER); if (fd < 0) return error_errno(_("could not lock '%s'"), todo_path); offset = get_item_line_offset(todo_list, next); @@ -3877,7 +3879,8 @@ static int safe_append(const char *filename, const char *fmt, ...) va_list ap; struct lock_file lock = LOCK_INIT; int fd = hold_lock_file_for_update(&lock, filename, - LOCK_REPORT_ON_ERROR); + LOCK_REPORT_ON_ERROR, + LOCKFILE_PID_OTHER); struct strbuf buf = STRBUF_INIT; if (fd < 0) @@ -4400,7 +4403,7 @@ static int write_update_refs_state(struct string_list *refs_to_oids) goto cleanup; } - if (hold_lock_file_for_update(&lock, path, 0) < 0) { + if (hold_lock_file_for_update(&lock, path, 0, LOCKFILE_PID_OTHER) < 0) { result = error(_("another 'rebase' process appears to be running; " "'%s.lock' already exists"), path); diff --git a/shallow.c b/shallow.c index 55b9cd9d3f29f1..412c37c96a72cb 100644 --- a/shallow.c +++ b/shallow.c @@ -392,7 +392,8 @@ void setup_alternate_shallow(struct shallow_lock *shallow_lock, fd = hold_lock_file_for_update(&shallow_lock->lock, git_path_shallow(the_repository), - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, + LOCKFILE_PID_SHALLOW); check_shallow_file_for_update(the_repository); if (write_shallow_commits(&sb, 0, extra)) { if (write_in_full(fd, sb.buf, sb.len) < 0) @@ -447,7 +448,8 @@ void prune_shallow(unsigned options) } fd = hold_lock_file_for_update(&shallow_lock.lock, git_path_shallow(the_repository), - LOCK_DIE_ON_ERROR); + LOCK_DIE_ON_ERROR, + LOCKFILE_PID_SHALLOW); check_shallow_file_for_update(the_repository); if (write_shallow_commits_1(&sb, 0, NULL, flags)) { if (write_in_full(fd, sb.buf, sb.len) < 0) diff --git a/t/meson.build b/t/meson.build index dc43d69636d15c..9a880db57c75b1 100644 --- a/t/meson.build +++ b/t/meson.build @@ -98,6 +98,7 @@ integration_tests = [ 't0028-working-tree-encoding.sh', 't0029-core-unsetenvvars.sh', 't0030-stripspace.sh', + 't0031-lockfile-pid.sh', 't0033-safe-directory.sh', 't0034-root-safe-directory.sh', 't0035-safe-bare-repository.sh', diff --git a/t/t0031-lockfile-pid.sh b/t/t0031-lockfile-pid.sh new file mode 100755 index 00000000000000..5f3a59b6fc3072 --- /dev/null +++ b/t/t0031-lockfile-pid.sh @@ -0,0 +1,119 @@ +#!/bin/sh + +test_description='lock file PID info tests + +Tests for PID info file alongside lock files. +The feature is opt-in via core.lockfilePid config setting. +' + +. ./test-lib.sh + +test_expect_success 'stale lock detected when PID is not running' ' + git init repo && + ( + cd repo && + touch .git/index.lock && + echo "99999" >.git/index-pid.lock && + test_must_fail git -c core.lockfilePid=index add . 2>err && + test_grep "process 99999, which is no longer running" err && + test_grep "Remove the stale lock file" err + ) +' + +test_expect_success 'PID info not shown by default' ' + git init repo2 && + ( + cd repo2 && + touch .git/index.lock && + echo "99999" >.git/index-pid.lock && + test_must_fail git add . 2>err && + # Should not crash, just show normal error without PID + test_grep "Unable to create" err && + ! test_grep "is held by process" err + ) +' + +test_expect_success 'running process detected when PID is alive' ' + git init repo3 && + ( + cd repo3 && + echo content >file && + # Create a lock and PID file with current shell PID (which is running) + touch .git/index.lock && + echo $$ >.git/index-pid.lock && + # Verify our PID is shown in the error message + test_must_fail git -c core.lockfilePid=index add file 2>err && + test_grep "held by process $$" err + ) +' + +test_expect_success 'PID info file cleaned up on successful operation when enabled' ' + git init repo4 && + ( + cd repo4 && + echo content >file && + git -c core.lockfilePid=index add file && + # After successful add, no lock or PID files should exist + test_path_is_missing .git/index.lock && + test_path_is_missing .git/index-pid.lock + ) +' + +test_expect_success 'no PID file created by default' ' + git init repo5 && + ( + cd repo5 && + echo content >file && + git add file && + # PID file should not be created when feature is disabled + test_path_is_missing .git/index-pid.lock + ) +' + +test_expect_success 'core.lockfilePid=all enables for all components' ' + git init repo6 && + ( + cd repo6 && + touch .git/index.lock && + echo "99999" >.git/index-pid.lock && + test_must_fail git -c core.lockfilePid=all add . 2>err && + test_grep "process 99999" err + ) +' + +test_expect_success 'multiple components can be specified' ' + git init repo8 && + ( + cd repo8 && + touch .git/index.lock && + echo "99999" >.git/index-pid.lock && + test_must_fail git -c core.lockfilePid=index,config add . 2>err && + test_grep "process 99999" err + ) +' + +test_expect_success 'core.lockfilePid=none does not create PID file' ' + git init repo9 && + ( + cd repo9 && + echo content >file && + git -c core.lockfilePid=none add file && + # PID file should not be created when feature is disabled + test_path_is_missing .git/index-pid.lock + ) +' + +test_expect_success 'existing PID files are read even when feature disabled' ' + git init repo10 && + ( + cd repo10 && + touch .git/index.lock && + echo "99999" >.git/index-pid.lock && + # Even with lockfilePid disabled, existing PID files are read + # to help diagnose stale locks + test_must_fail git add . 2>err && + test_grep "process 99999" err + ) +' + +test_done diff --git a/unix-stream-server.c b/unix-stream-server.c index 22ac2373e0788d..d8c79cd569bbd5 100644 --- a/unix-stream-server.c +++ b/unix-stream-server.c @@ -47,7 +47,8 @@ int unix_ss_create(const char *path, /* * Create a lock at ".lock" if we can. */ - if (hold_lock_file_for_update_timeout(&lock, path, 0, timeout_ms) < 0) + if (hold_lock_file_for_update_timeout(&lock, path, 0, timeout_ms, + LOCKFILE_PID_OTHER) < 0) return -1; /*