From 83c6040067e2aa28e360483a38ca26161bd0f9a9 Mon Sep 17 00:00:00 2001 From: Koji Nakamaru Date: Wed, 2 Oct 2024 17:49:51 +0900 Subject: [PATCH] fsmonitor: fix hangs by delayed fs event listening The thread serving the client (ipc-thread) calls with_lock__wait_for_cookie() in which a cookie file is created. with_lock__wait_for_cookie() then waits for the event caused by the cookie file from the thread for fs events (fsevent-thread). However, in high load situations, the fsevent-thread may start actual fs event listening (triggered by FSEventStreamStart() for Darwin, for example) *after* the cookie file is created. In this case, the fsevent-thread cannot detect the cookie file and with_lock__wait_for_cookie() waits forever, so that the whole daemon hangs [1]. Extend listen_error_code to express that actual fs event listening starts. listen_error_code is accessed in a thread-safe manner by utilizing a dedicated mutex. [1]: https://lore.kernel.org/git/20241002062539.GA2863841@coredump.intra.peff.net/ Suggested-by: Jeff King Signed-off-by: Koji Nakamaru --- builtin/fsmonitor--daemon.c | 21 +++++++++++++++++++++ compat/fsmonitor/fsm-listen-darwin.c | 5 +++-- compat/fsmonitor/fsm-listen-win32.c | 5 ++--- fsmonitor--daemon.h | 4 ++++ 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index dce8a3b248291d..ccff2cb8bed959 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -172,6 +172,9 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie( trace_printf_key(&trace_fsmonitor, "cookie-wait: '%s' '%s'", cookie->name, cookie_pathname.buf); + while (fsmonitor_get_listen_error_code(state) == 0) + sleep_millisec(50); + /* * Create the cookie file on disk and then wait for a notification * that the listener thread has seen it. @@ -606,6 +609,23 @@ void fsmonitor_force_resync(struct fsmonitor_daemon_state *state) pthread_mutex_unlock(&state->main_lock); } +int fsmonitor_get_listen_error_code(struct fsmonitor_daemon_state *state) +{ + int error_code; + + pthread_mutex_lock(&state->listen_lock); + error_code = state->listen_error_code; + pthread_mutex_unlock(&state->listen_lock); + return error_code; +} + +void fsmonitor_set_listen_error_code(struct fsmonitor_daemon_state *state, int error_code) +{ + pthread_mutex_lock(&state->listen_lock); + state->listen_error_code = error_code; + pthread_mutex_unlock(&state->listen_lock); +} + /* * Format an opaque token string to send to the client. */ @@ -1285,6 +1305,7 @@ static int fsmonitor_run_daemon(void) hashmap_init(&state.cookies, cookies_cmp, NULL, 0); pthread_mutex_init(&state.main_lock, NULL); pthread_cond_init(&state.cookies_cond, NULL); + pthread_mutex_init(&state.listen_lock, NULL); state.listen_error_code = 0; state.health_error_code = 0; state.current_token_data = fsmonitor_new_token_data(); diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c index 2fc67442eb5e87..92373ce247fc83 100644 --- a/compat/fsmonitor/fsm-listen-darwin.c +++ b/compat/fsmonitor/fsm-listen-darwin.c @@ -515,6 +515,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) goto force_error_stop_without_loop; } data->stream_started = 1; + fsmonitor_set_listen_error_code(state, 1); pthread_mutex_lock(&data->dq_lock); pthread_cond_wait(&data->dq_finished, &data->dq_lock); @@ -522,7 +523,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) switch (data->shutdown_style) { case FORCE_ERROR_STOP: - state->listen_error_code = -1; + fsmonitor_set_listen_error_code(state, -1); /* fall thru */ case FORCE_SHUTDOWN: ipc_server_stop_async(state->ipc_server_data); @@ -534,7 +535,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) return; force_error_stop_without_loop: - state->listen_error_code = -1; + fsmonitor_set_listen_error_code(state, -1); ipc_server_stop_async(state->ipc_server_data); return; } diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c index 5a21dade7b8659..609efd1d6a8f3e 100644 --- a/compat/fsmonitor/fsm-listen-win32.c +++ b/compat/fsmonitor/fsm-listen-win32.c @@ -732,14 +732,13 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) DWORD dwWait; int result; - state->listen_error_code = 0; - if (start_rdcw_watch(data->watch_worktree) == -1) goto force_error_stop; if (data->watch_gitdir && start_rdcw_watch(data->watch_gitdir) == -1) goto force_error_stop; + fsmonitor_set_listen_error_code(state, 1); for (;;) { dwWait = WaitForMultipleObjects(data->nr_listener_handles, @@ -797,7 +796,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) } force_error_stop: - state->listen_error_code = -1; + fsmonitor_set_listen_error_code(state, -1); force_shutdown: /* diff --git a/fsmonitor--daemon.h b/fsmonitor--daemon.h index 5cbbec8d940ba7..77efc59b7bb040 100644 --- a/fsmonitor--daemon.h +++ b/fsmonitor--daemon.h @@ -51,6 +51,7 @@ struct fsmonitor_daemon_state { int cookie_seq; struct hashmap cookies; + pthread_mutex_t listen_lock; int listen_error_code; int health_error_code; struct fsm_listen_data *listen_data; @@ -167,5 +168,8 @@ void fsmonitor_publish(struct fsmonitor_daemon_state *state, */ void fsmonitor_force_resync(struct fsmonitor_daemon_state *state); +int fsmonitor_get_listen_error_code(struct fsmonitor_daemon_state *state); +void fsmonitor_set_listen_error_code(struct fsmonitor_daemon_state *state, int error_code); + #endif /* HAVE_FSMONITOR_DAEMON_BACKEND */ #endif /* FSMONITOR_DAEMON_H */