Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait

On Tue, Jul 21, 2026 at 05:04:56PM -0400, Tamir Duberstein wrote:
> 56cef9cb1a (fsmonitor: use pthread_cond_timedwait for cookie wait,
> 2026-04-15) limits the cookie wait to one second so that a filesystem
> which never delivers events cannot hang fsmonitor clients. A client that
> times out receives a trivial response and scans the entire index.
> 
> FSEvents can defer delivery while it batches notifications and does not
> guarantee that its queue is drained in one latency interval. A loaded
> macOS system can therefore time out even though the event stream is
> working.
> 
> On an Apple M4 Max (16 cores, 128 GiB RAM) running macOS 26.5.2, two
> worktrees with a 1,001,178-entry index timed out 484 of 545 and 297 of
> 365 fsmonitor requests. One status call performed 934,519 lstat() calls
> during a 47-second preload and took 52 seconds overall.
> 
> Ask FSEvents to flush pending notifications after creating the cookie
> and before starting the timed wait. Use the asynchronous form because
> the client handler holds main_lock, which the listener callback also
> acquires. Keep the timeout and the behavior of the other backends
> unchanged.

I cannot really say much about the FSEvent interfaces, but to me it
feels quite reasonable to flush the queue when we are waiting for events
to be delivered. And that's exactly what `FSEventStreamFlushAsync()`
does: it basically overrides the latency we have configured (which is
1ms) and asks the kernel to flush stuff immediately.

> diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
> index 4161dd8282..8e32b5ae5e 100644
> --- a/builtin/fsmonitor--daemon.c
> +++ b/builtin/fsmonitor--daemon.c
> @@ -206,6 +206,9 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie(
>  	close(fd);
>  	unlink(cookie_pathname.buf);
>  
> +	/* The listener callback takes main_lock, so this must not block. */
> +	fsm_listen__flush_async(state);
> +
>  	/*
>  	 * Wait for the listener thread to observe the cookie file.
>  	 * Time out after a short interval so that the client

Okay, so we've unlinked the cookie file and the next thing is that we're
waiting for all events to have been processed. As said, it feels
reasonable that we're flushing all events before we start waiting for
them.

What I find surprising though is that this is supposed to make a
difference at all. The latency we pass to `FSEventStreamCreate()` is
1 millisecond, and we wait up to 1 second for the cookie event. I would
have expected that batching events for 1 milliseconds should be totally
fine when we're waiting for a full second anyway.

So given that I cannot verify this at all and that I have no clue about
the FSEvent interfaces... do you have any explanation why the flush
seems to help regardless?

I _think_ you're already hinting at this in the commit message, where
you say that it's not guaranteed that the queue is drained in a single
latency interval. Is there any documentation that tells us what the
provided guarantees are?

Other than that the code changes look sensible to me, thanks!

Patrick