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 <[email protected]>
microsoft · Apr 25, 2023 · 6f5e96b · 6f5e96b
1 parent 9dee1ff
commit 6f5e96b
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 0 deletions.
diff --git a/builtin/gc.c b/builtin/gc.c
@@ -1304,6 +1304,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.
 		 *
@@ -1314,6 +1316,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
@@ -54,6 +54,23 @@ test_expect_success 'run [--auto|--quiet]' '
 	test_subcommand git gc --no-quiet <run-no-quiet.txt
 '
 
+test_expect_success 'lock file behavior' '
+	test_when_finished git config --unset maintenance.commit-graph.schedule &&
+	git config maintenance.commit-graph.schedule hourly &&
+
+	touch .git/objects/maintenance.lock &&
+	git maintenance run --schedule=hourly --no-quiet 2>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 <default &&