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

On Wed, Aug 5, 2026 at 3:59 AM Patrick Steinhardt <[email protected]> wrote:
>
> 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

The following was generated by my coding agent and fact checked and
edited by me mainly to address you in the second person.

Your question was already answered by the original Git implementation
- and you yourself predicted this exact regression before it landed.

In March 2022, Jeff Hostetler introduced Git’s fsmonitor cookie
protocol in commit b05880d357. Its commit message explicitly says
macOS “does not guarantee that the kernel queue is completely drained”
after one FSEvents latency interval. That is precisely why Git
originally waited until it actually observed the cookie. Original
cookie implementation
(https://github.com/git/git/commit/b05880d357c6dadba8d1d7943f4782fc25e06999)

Regression timeline:

1. February 2026: Paul Tarjan proposed replacing the indefinite cookie
wait with a one-second timeout to prevent hangs on Linux filesystems
that never deliver events. Junio questioned whether one second was
appropriate and warned about expensive full-scan fallbacks. Junio’s
initial concern
(https://lore.kernel.org/git/[email protected]/); Junio’s
full-scan warning
(https://lore.kernel.org/git/[email protected]/)
2. Paul’s assumption: He argued that the timeout would trigger only on
broken filesystems that never deliver events, while working
filesystems would respond promptly. Paul’s explanation
(https://lore.kernel.org/git/[email protected]/)
3. March 4: You (Patrick) asked: “Are we sure this is always enough on
a loaded system?” Paul responded that even if a timeout occurred, the
fallback would simply involve some additional work. Patrick’s earlier
warning (https://lore.kernel.org/git/[email protected]/); Paul’s
response (https://lore.kernel.org/git/[email protected]/)
4. April 15: The one-second timeout landed anyway as 56cef9cb1a.
Accepted timeout change
(https://github.com/git/git/commit/56cef9cb1a083c47b12b88548bf2126af8bfb263)
5. July 21: My (tamird) measurements disproved both assumptions: 781
of 910 requests timed out on functioning macOS worktrees, and one
fallback caused 934,519 lstat() calls and a 52-second git status. The
result is this patch.

Summary:

The 1 ms value is not a delivery deadline:

- Apple defines it as the delay the userspace service should apply
after it hears about an event from the kernel. It says nothing about
kernel backlog, service scheduling, callback scheduling, or complete
queue drainage. The installed SDK spells this out in
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreServices.framework/Frameworks/FSEvents.framework/Headers/FSEvents.h:763.

 *    latency:
 *      The number of seconds the service should wait after hearing
 *      about an event from the kernel before passing it along to the
 *      client via its callback. Specifying a larger value may result
 *      in more effective temporal coalescing, resulting in fewer
 *      callbacks and greater overall efficiency.

- Apple explicitly describes notification latency as “inherently
non-deterministic.” Apple’s FSEvents programming guide
(https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/FSEvents_ProgGuide/UsingtheFSEventsFramework/UsingtheFSEventsFramework.html)
- Apple’s kernel independently implements a 10 ms event-batching
timer, demonstrating that the userspace 1 ms parameter is not even the
only batching interval. This does not itself explain a one-second
delay; it disproves treating 1 ms as an end-to-end guarantee. Apple
XNU FSEvents implementation
(https://github.com/apple-oss-distributions/xnu/blob/f6217f891ac0bb64f3d375211650a4c1ff8ca1ea/bsd/vfs/vfs_fsevents.c#L1479-L1525)
- Git’s original Darwin implementation chose 1 ms because 100 ms
caused dropped events in a 100,000-file stress test—not because Apple
guaranteed delivery within 1 ms. See
compat/fsmonitor/fsm-listen-darwin.c:437.
- FSEventStreamFlushAsync() requests delivery of pending events
without blocking. A synchronous flush at the existing call site would
deadlock because the caller already holds the mutex needed by the
callback. See builtin/fsmonitor--daemon.c:247.

Hope that's helpful.