From bbceaa865ed488ec97b060056c46d70cd5cb9a22 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 16 Nov 2021 10:26:08 -0500 Subject: [PATCH 1/2] maintenance: delete stale lock files The maintenance.lock file exists to prevent concurrent maintenance processes from writing to a repository at the same time. However, it has the downside of causing maintenance to start failing without recovery if there is any reason why the maintenance command failed without cleaning up the lock-file. This change makes it such that maintenance will delete a lock file that was modified over 6 hours ago. This will auto-heal repositories that are stuck with failed maintenance (and maybe it will fail again, but we will get a message other than the lock file exists). Signed-off-by: Derrick Stolee --- builtin/gc.c | 21 +++++++++++++++++++++ t/t7900-maintenance.sh | 17 +++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index e9f883658dfa75..819d16e4300be4 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1274,6 +1274,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts) char *lock_path = xstrfmt("%s/maintenance", r->objects->odb->path); if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) { + struct stat st; + struct strbuf lock_dot_lock = STRBUF_INIT; /* * Another maintenance command is running. * @@ -1284,6 +1286,25 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts) if (!opts->auto_flag && !opts->quiet) warning(_("lock file '%s' exists, skipping maintenance"), lock_path); + + /* + * Check timestamp on .lock file to see if we should + * delete it to recover from a fail state. + */ + strbuf_addstr(&lock_dot_lock, lock_path); + strbuf_addstr(&lock_dot_lock, ".lock"); + if (lstat(lock_dot_lock.buf, &st)) + warning_errno(_("unable to stat '%s'"), lock_dot_lock.buf); + else { + if (st.st_mtime < time(NULL) - (6 * 60 * 60)) { + if (unlink(lock_dot_lock.buf)) + warning_errno(_("unable to delete stale lock file")); + else + warning(_("deleted stale lock file")); + } + } + + strbuf_release(&lock_dot_lock); free(lock_path); return 0; } diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 74aa6384755ec6..0ab49b0eccbf68 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -52,6 +52,23 @@ test_expect_success 'run [--auto|--quiet]' ' test_subcommand git gc --no-quiet err && + grep "lock file .* exists, skipping maintenance" err && + + test-tool chmtime =-22000 .git/objects/maintenance.lock && + git maintenance run --schedule=hourly --no-quiet 2>err && + grep "deleted stale lock file" err && + test_path_is_missing .git/objects/maintenance.lock && + + git maintenance run --schedule=hourly 2>err && + test_must_be_empty err +' + test_expect_success 'maintenance.auto config option' ' GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 && test_subcommand git maintenance run --auto --quiet Date: Tue, 16 Nov 2021 11:19:31 -0500 Subject: [PATCH 2/2] fixup! maintenance: care about gvfs.sharedCache config The bug here is that object_dir was being set to an internal pointer within git_config_get_value(), but then getting clobbered by the stack or something. The xstrdup() here ensures that we don't lose the value this way. Signed-off-by: Derrick Stolee --- builtin/gc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 819d16e4300be4..25e5bffcb4ec78 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1433,6 +1433,7 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) { int i; struct maintenance_run_opts opts; + const char *tmp_obj_dir = NULL; struct option builtin_maintenance_run_options[] = { OPT_BOOL(0, "auto", &opts.auto_flag, N_("run tasks based on the state of the repository")), @@ -1472,9 +1473,11 @@ static int maintenance_run(int argc, const char **argv, const char *prefix) * the gvfs.sharedcache config option to redirect the * maintenance to that location. */ - if (!git_config_get_value("gvfs.sharedcache", &object_dir) && - object_dir) + if (!git_config_get_value("gvfs.sharedcache", &tmp_obj_dir) && + tmp_obj_dir) { + object_dir = xstrdup(tmp_obj_dir); setenv(DB_ENVIRONMENT, object_dir, 1); + } return maintenance_run_tasks(&opts); }