Re: [PATCH v6 04/10] fsmonitor: use pthread_cond_timedwait for cookie wait
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 25, 2026, 21:13 UTC
- Message-ID
- <xmqqzf4w8r20.fsf@gitster.g>
- In-Reply-To
- <0051a1930349878fd25bb5d2240073beef36da7d.1772050636.git.gitgitgadget@gmail.com>
"Paul Tarjan via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 16 quoted lines
> From: Paul Tarjan <github@paulisageek.com> > > The cookie wait in with_lock__wait_for_cookie() uses an infinite > pthread_cond_wait() loop. The existing comment notes the desire > to switch to pthread_cond_timedwait(), but the routine was not > available in git thread-utils. > > On certain container or overlay filesystems, inotify watches may > succeed but events are never delivered. In this case the daemon > would hang indefinitely waiting for the cookie event, which in > turn causes the client to hang. > > Replace the infinite wait with a one-second timeout using > pthread_cond_timedwait(). If the timeout fires, report an > error and let the client proceed with a trivial (full-scan) > response rather than blocking forever.
I cannot convince myself if one-second interval is not too frequent to force everybody, including those with working inotify, to poll. I wonder if this is something that may want to be configurable (or better yet, auto-detectable, but that may be wishing for moon).
Show 54 quoted lines
> Signed-off-by: Paul Tarjan <github@paulisageek.com>
> ---
> builtin/fsmonitor--daemon.c | 37 ++++++++++++++++++++++++-------------
> 1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
> index 4d52622e24..f6c406ff12 100644
> --- a/builtin/fsmonitor--daemon.c
> +++ b/builtin/fsmonitor--daemon.c
> @@ -197,20 +197,31 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie(
> unlink(cookie_pathname.buf);
>
> /*
> - * Technically, this is an infinite wait (well, unless another
> - * thread sends us an abort). I'd like to change this to
> - * use `pthread_cond_timedwait()` and return an error/timeout
> - * and let the caller do the trivial response thing, but we
> - * don't have that routine in our thread-utils.
> - *
> - * After extensive beta testing I'm not really worried about
> - * this. Also note that the above open() and unlink() calls
> - * will cause at least two FS events on that path, so the odds
> - * of getting stuck are pretty slim.
> + * Wait for the listener thread to see the cookie file.
> + * Time out after a short interval so that the client
> + * does not hang forever if the filesystem does not deliver
> + * events (e.g., on certain container/overlay filesystems
> + * where inotify watches succeed but events never arrive).
> */
> - while (cookie->result == FCIR_INIT)
> - pthread_cond_wait(&state->cookies_cond,
> - &state->main_lock);
> + {
> + struct timeval now;
> + struct timespec ts;
> + int err = 0;
> +
> + gettimeofday(&now, NULL);
> + ts.tv_sec = now.tv_sec + 1;
> + ts.tv_nsec = now.tv_usec * 1000;
> +
> + while (cookie->result == FCIR_INIT && !err)
> + err = pthread_cond_timedwait(&state->cookies_cond,
> + &state->main_lock,
> + &ts);
> + if (err == ETIMEDOUT && cookie->result == FCIR_INIT) {
> + trace_printf_key(&trace_fsmonitor,
> + "cookie_wait timed out");
> + cookie->result = FCIR_ERROR;
> + }
> + }
>
> done:
> hashmap_remove(&state->cookies, &cookie->entry, NULL);