[PATCH v4] fsmonitor OSX: fix hangs for submodules

From: Koji Nakamaru via GitGitGadget <gitgitgadget@gmail.com>
Date: Oct 4, 2024, 00:07 UTC
Message-ID: <pull.1802.v4.git.1728000433326.gitgitgadget@gmail.com>
In-Reply-To: <pull.1802.v3.git.1727810964571.gitgitgadget@gmail.com>

From: Koji Nakamaru <koji.nakamaru@gree.net>

fsmonitor_classify_path_absolute() expects state->path_gitdir_watch.buf has no trailing '/' or '.' For a submodule, fsmonitor_run_daemon() sets the value with trailing "/." (as repo_get_git_dir(the_repository) on Darwin returns ".") so that fsmonitor_classify_path_absolute() returns IS_OUTSIDE_CONE.

In this case, fsevent_callback() doesn't update cookie_list so that fsmonitor_publish() does nothing and with_lock__mark_cookies_seen() is not invoked.

As with_lock__wait_for_cookie() infinitely waits for state->cookies_cond that with_lock__mark_cookies_seen() should unlock, the whole daemon hangs.

Remove trailing "/." from state->path_gitdir_watch.buf for submodules and add a corresponding test in t7527-builtin-fsmonitor.sh. The test is disabled for MINGW because hangs treated with this patch occur only for Darwin and there is no simple way to terminate the win32 fsmonitor daemon that hangs.

Suggested-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net>
---
    fsmonitor/darwin: fix hangs for submodules

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1802%2FKojiNakamaru%2Ffix%2Ffsmonitor-darwin-hangs-for-submodules-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1802/KojiNakamaru/fix/fsmonitor-darwin-hangs-for-submodules-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1802

Range-diff vs v3:

 1:  aabc1c2f6ee ! 1:  7b7224ef718 fsmonitor OSX: fix hangs for submodules
     @@ Commit message
          hangs.
      
          Remove trailing "/." from state->path_gitdir_watch.buf for submodules
     -    and add a corresponding test in t7527-builtin-fsmonitor.sh.
     +    and add a corresponding test in t7527-builtin-fsmonitor.sh. The test is
     +    disabled for MINGW because hangs treated with this patch occur only for
     +    Darwin and there is no simple way to terminate the win32 fsmonitor
     +    daemon that hangs.
      
          Suggested-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Suggested-by: Junio C Hamano <gitster@pobox.com>
     @@ t/t7527-builtin-fsmonitor.sh: test_expect_success "submodule absorbgitdirs impli
      +	done
      +}
      +
     -+test_expect_success "submodule implicitly starts daemon by pull" '
     ++test_expect_success !MINGW "submodule implicitly starts daemon by pull" '
      +	test_atexit "stop_watchdog" &&
      +	test_when_finished "stop_git; rm -rf cloned super sub" &&
      +

 builtin/fsmonitor--daemon.c  |  1 +
 t/t7527-builtin-fsmonitor.sh | 51 ++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index dce8a3b2482..e1e6b96d09e 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -1314,6 +1314,7 @@ static int fsmonitor_run_daemon(void)
 		strbuf_reset(&state.path_gitdir_watch);
 		strbuf_addstr(&state.path_gitdir_watch,
 			      absolute_path(repo_get_git_dir(the_repository)));
+		strbuf_strip_suffix(&state.path_gitdir_watch, "/.");
 		state.nr_paths_watching = 2;
 	}
 
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 730f3c7f810..9b15baa02d3 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -907,6 +907,57 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
 	test_subcommand git fsmonitor--daemon start <super-sub.trace
 '
 
+start_git_in_background () {
+	git "$@" &
+	git_pid=$!
+	git_pgid=$(ps -o pgid= -p $git_pid)
+	nr_tries_left=10
+	while true
+	do
+		if test $nr_tries_left -eq 0
+		then
+			kill -- -$git_pgid
+			exit 1
+		fi
+		sleep 1
+		nr_tries_left=$(($nr_tries_left - 1))
+	done >/dev/null 2>&1 &
+	watchdog_pid=$!
+	wait $git_pid
+}
+
+stop_git () {
+	while kill -0 -- -$git_pgid
+	do
+		kill -- -$git_pgid
+		sleep 1
+	done
+}
+
+stop_watchdog () {
+	while kill -0 $watchdog_pid
+	do
+		kill $watchdog_pid
+		sleep 1
+	done
+}
+
+test_expect_success !MINGW "submodule implicitly starts daemon by pull" '
+	test_atexit "stop_watchdog" &&
+	test_when_finished "stop_git; rm -rf cloned super sub" &&
+
+	create_super super &&
+	create_sub sub &&
+
+	git -C super submodule add ../sub ./dir_1/dir_2/sub &&
+	git -C super commit -m "add sub" &&
+	git clone --recurse-submodules super cloned &&
+
+	git -C cloned/dir_1/dir_2/sub config core.fsmonitor true &&
+	set -m &&
+	start_git_in_background -C cloned pull --recurse-submodules
+'
+
 # On a case-insensitive file system, confirm that the daemon
 # notices when the .git directory is moved/renamed/deleted
 # regardless of how it is spelled in the FS event.

base-commit: 3857aae53f3633b7de63ad640737c657387ae0c6

-- 
gitgitgadget