* [PATCH] fsmonitor: flush pending FSEvents before cookie wait
@ 2026-07-21 21:04 Tamir Duberstein
2026-07-24 2:41 ` Koji Nakamaru
2026-08-05 7:59 ` Patrick Steinhardt
0 siblings, 2 replies; 9+ messages in thread
From: Tamir Duberstein @ 2026-07-21 21:04 UTC (permalink / raw)
To: git
Cc: Jeff Hostetler, Paul Tarjan, Patrick Steinhardt, Junio C Hamano,
Jeff King, Taylor Blau, Johannes Schindelin, Koji Nakamaru,
Tamir Duberstein
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.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
builtin/fsmonitor--daemon.c | 3 +++
compat/fsmonitor/fsm-darwin-gcc.h | 1 +
compat/fsmonitor/fsm-listen-darwin.c | 5 +++++
compat/fsmonitor/fsm-listen-linux.c | 4 ++++
compat/fsmonitor/fsm-listen-win32.c | 4 ++++
compat/fsmonitor/fsm-listen.h | 6 ++++++
6 files changed, 23 insertions(+)
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
diff --git a/compat/fsmonitor/fsm-darwin-gcc.h b/compat/fsmonitor/fsm-darwin-gcc.h
index 3496e29b3a..c209dc2f68 100644
--- a/compat/fsmonitor/fsm-darwin-gcc.h
+++ b/compat/fsmonitor/fsm-darwin-gcc.h
@@ -82,6 +82,7 @@ CFRunLoopRef CFRunLoopGetCurrent(void);
extern CFStringRef kCFRunLoopDefaultMode;
void FSEventStreamSetDispatchQueue(FSEventStreamRef stream, dispatch_queue_t q);
unsigned char FSEventStreamStart(FSEventStreamRef stream);
+FSEventStreamEventId FSEventStreamFlushAsync(FSEventStreamRef stream);
void FSEventStreamStop(FSEventStreamRef stream);
void FSEventStreamInvalidate(FSEventStreamRef stream);
void FSEventStreamRelease(FSEventStreamRef stream);
diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c
index 43c3a915a0..64bee248d2 100644
--- a/compat/fsmonitor/fsm-listen-darwin.c
+++ b/compat/fsmonitor/fsm-listen-darwin.c
@@ -496,6 +496,11 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)
pthread_mutex_unlock(&data->dq_lock);
}
+void fsm_listen__flush_async(struct fsmonitor_daemon_state *state)
+{
+ FSEventStreamFlushAsync(state->listen_data->stream);
+}
+
void fsm_listen__loop(struct fsmonitor_daemon_state *state)
{
struct fsm_listen_data *data;
diff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c
index e3dca14b62..7aae29ea22 100644
--- a/compat/fsmonitor/fsm-listen-linux.c
+++ b/compat/fsmonitor/fsm-listen-linux.c
@@ -493,6 +493,10 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)
state->listen_data->shutdown = SHUTDOWN_STOP;
}
+void fsm_listen__flush_async(struct fsmonitor_daemon_state *state UNUSED)
+{
+}
+
/*
* Process a single inotify event and queue for publication.
*/
diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c
index 9a6efc9bea..039d797000 100644
--- a/compat/fsmonitor/fsm-listen-win32.c
+++ b/compat/fsmonitor/fsm-listen-win32.c
@@ -290,6 +290,10 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)
SetEvent(state->listen_data->hListener[LISTENER_SHUTDOWN]);
}
+void fsm_listen__flush_async(struct fsmonitor_daemon_state *state UNUSED)
+{
+}
+
static struct one_watch *create_watch(const char *path)
{
struct one_watch *watch = NULL;
diff --git a/compat/fsmonitor/fsm-listen.h b/compat/fsmonitor/fsm-listen.h
index 41650bf897..cfeca1f4b6 100644
--- a/compat/fsmonitor/fsm-listen.h
+++ b/compat/fsmonitor/fsm-listen.h
@@ -38,6 +38,12 @@ void fsm_listen__dtor(struct fsmonitor_daemon_state *state);
*/
void fsm_listen__loop(struct fsmonitor_daemon_state *state);
+/*
+ * Prompt the listener to deliver queued filesystem events, if supported.
+ * This does not wait for the events to be processed.
+ */
+void fsm_listen__flush_async(struct fsmonitor_daemon_state *state);
+
/*
* Gently request that the fsmonitor listener thread shutdown.
* It does not wait for it to stop. The caller should do a JOIN
---
base-commit: 5d2e7709234afea1b6ddb25cd4f60d3d5fb3c200
change-id: 20260721-fsmonitor-darwin-cookie-flush-0f0d6e554a56
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-07-21 21:04 [PATCH] fsmonitor: flush pending FSEvents before cookie wait Tamir Duberstein
@ 2026-07-24 2:41 ` Koji Nakamaru
2026-07-24 20:38 ` Junio C Hamano
2026-08-04 22:13 ` Junio C Hamano
2026-08-05 7:59 ` Patrick Steinhardt
1 sibling, 2 replies; 9+ messages in thread
From: Koji Nakamaru @ 2026-07-24 2:41 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Jeff Hostetler, Paul Tarjan, Patrick Steinhardt,
Junio C Hamano, Jeff King, Taylor Blau, Johannes Schindelin
On Wed, Jul 22, 2026 at 6:05 AM Tamir Duberstein <tamird@gmail.com> 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.
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> builtin/fsmonitor--daemon.c | 3 +++
> compat/fsmonitor/fsm-darwin-gcc.h | 1 +
> compat/fsmonitor/fsm-listen-darwin.c | 5 +++++
> compat/fsmonitor/fsm-listen-linux.c | 4 ++++
> compat/fsmonitor/fsm-listen-win32.c | 4 ++++
> compat/fsmonitor/fsm-listen.h | 6 ++++++
> 6 files changed, 23 insertions(+)
>
> 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
> diff --git a/compat/fsmonitor/fsm-darwin-gcc.h b/compat/fsmonitor/fsm-darwin-gcc.h
> index 3496e29b3a..c209dc2f68 100644
> --- a/compat/fsmonitor/fsm-darwin-gcc.h
> +++ b/compat/fsmonitor/fsm-darwin-gcc.h
> @@ -82,6 +82,7 @@ CFRunLoopRef CFRunLoopGetCurrent(void);
> extern CFStringRef kCFRunLoopDefaultMode;
> void FSEventStreamSetDispatchQueue(FSEventStreamRef stream, dispatch_queue_t q);
> unsigned char FSEventStreamStart(FSEventStreamRef stream);
> +FSEventStreamEventId FSEventStreamFlushAsync(FSEventStreamRef stream);
> void FSEventStreamStop(FSEventStreamRef stream);
> void FSEventStreamInvalidate(FSEventStreamRef stream);
> void FSEventStreamRelease(FSEventStreamRef stream);
> diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c
> index 43c3a915a0..64bee248d2 100644
> --- a/compat/fsmonitor/fsm-listen-darwin.c
> +++ b/compat/fsmonitor/fsm-listen-darwin.c
> @@ -496,6 +496,11 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)
> pthread_mutex_unlock(&data->dq_lock);
> }
>
> +void fsm_listen__flush_async(struct fsmonitor_daemon_state *state)
> +{
> + FSEventStreamFlushAsync(state->listen_data->stream);
> +}
> +
> void fsm_listen__loop(struct fsmonitor_daemon_state *state)
> {
> struct fsm_listen_data *data;
> diff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c
> index e3dca14b62..7aae29ea22 100644
> --- a/compat/fsmonitor/fsm-listen-linux.c
> +++ b/compat/fsmonitor/fsm-listen-linux.c
> @@ -493,6 +493,10 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)
> state->listen_data->shutdown = SHUTDOWN_STOP;
> }
>
> +void fsm_listen__flush_async(struct fsmonitor_daemon_state *state UNUSED)
> +{
> +}
> +
> /*
> * Process a single inotify event and queue for publication.
> */
> diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c
> index 9a6efc9bea..039d797000 100644
> --- a/compat/fsmonitor/fsm-listen-win32.c
> +++ b/compat/fsmonitor/fsm-listen-win32.c
> @@ -290,6 +290,10 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)
> SetEvent(state->listen_data->hListener[LISTENER_SHUTDOWN]);
> }
>
> +void fsm_listen__flush_async(struct fsmonitor_daemon_state *state UNUSED)
> +{
> +}
> +
> static struct one_watch *create_watch(const char *path)
> {
> struct one_watch *watch = NULL;
> diff --git a/compat/fsmonitor/fsm-listen.h b/compat/fsmonitor/fsm-listen.h
> index 41650bf897..cfeca1f4b6 100644
> --- a/compat/fsmonitor/fsm-listen.h
> +++ b/compat/fsmonitor/fsm-listen.h
> @@ -38,6 +38,12 @@ void fsm_listen__dtor(struct fsmonitor_daemon_state *state);
> */
> void fsm_listen__loop(struct fsmonitor_daemon_state *state);
>
> +/*
> + * Prompt the listener to deliver queued filesystem events, if supported.
> + * This does not wait for the events to be processed.
> + */
> +void fsm_listen__flush_async(struct fsmonitor_daemon_state *state);
> +
> /*
> * Gently request that the fsmonitor listener thread shutdown.
> * It does not wait for it to stop. The caller should do a JOIN
>
> ---
> base-commit: 5d2e7709234afea1b6ddb25cd4f60d3d5fb3c200
> change-id: 20260721-fsmonitor-darwin-cookie-flush-0f0d6e554a56
>
This patch is carefully designed to minimize any risks. To drain events,
we could also call FSEventStreamFlushSync before acquiring main_lock in
do_handle_client(), but this patch should be sufficient if it mitigates
the issue. The commit message would be much more convincing if you also
included benchmark results showing how many timeouts were reduced.
--
Koji Nakamaru
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-07-24 2:41 ` Koji Nakamaru
@ 2026-07-24 20:38 ` Junio C Hamano
2026-08-04 22:13 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2026-07-24 20:38 UTC (permalink / raw)
To: Koji Nakamaru
Cc: Tamir Duberstein, git, Jeff Hostetler, Paul Tarjan,
Patrick Steinhardt, Jeff King, Taylor Blau, Johannes Schindelin
Koji Nakamaru <koji.nakamaru@gree.net> writes:
> On Wed, Jul 22, 2026 at 6:05 AM Tamir Duberstein <tamird@gmail.com> 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.
>>
>> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> ---
>>...
> This patch is carefully designed to minimize any risks. To drain events,
> we could also call FSEventStreamFlushSync before acquiring main_lock in
> do_handle_client(), but this patch should be sufficient if it mitigates
> the issue. The commit message would be much more convincing if you also
> included benchmark results showing how many timeouts were reduced.
Thanks for a review.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-07-24 2:41 ` Koji Nakamaru
2026-07-24 20:38 ` Junio C Hamano
@ 2026-08-04 22:13 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2026-08-04 22:13 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Koji Nakamaru, git, Paul Tarjan, Patrick Steinhardt, Jeff King,
Taylor Blau, Johannes Schindelin
Koji Nakamaru <koji.nakamaru@gree.net> writes:
> On Wed, Jul 22, 2026 at 6:05 AM Tamir Duberstein <tamird@gmail.com> 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.
>> ...
>
> This patch is carefully designed to minimize any risks. To drain events,
> we could also call FSEventStreamFlushSync before acquiring main_lock in
> do_handle_client(), but this patch should be sufficient if it mitigates
> the issue. The commit message would be much more convincing if you also
> included benchmark results showing how many timeouts were reduced.
Tamir, just to say that it is my understanding that the ball is in
your court. It hasn't been _too_ long since the exchange happened,
but we expect people to respond review comments (either positively
or negatively) and without such discourse a topic would not move
forward, so ...
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-07-21 21:04 [PATCH] fsmonitor: flush pending FSEvents before cookie wait Tamir Duberstein
2026-07-24 2:41 ` Koji Nakamaru
@ 2026-08-05 7:59 ` Patrick Steinhardt
2026-08-11 15:22 ` Tamir Duberstein
1 sibling, 1 reply; 9+ messages in thread
From: Patrick Steinhardt @ 2026-08-05 7:59 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Jeff Hostetler, Paul Tarjan, Junio C Hamano, Jeff King,
Taylor Blau, Johannes Schindelin, Koji Nakamaru
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-08-05 7:59 ` Patrick Steinhardt
@ 2026-08-11 15:22 ` Tamir Duberstein
2026-08-11 16:23 ` Patrick Steinhardt
0 siblings, 1 reply; 9+ messages in thread
From: Tamir Duberstein @ 2026-08-11 15:22 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Jeff Hostetler, Paul Tarjan, Junio C Hamano, Jeff King,
Taylor Blau, Johannes Schindelin, Koji Nakamaru
On Wed, Aug 5, 2026 at 3:59 AM Patrick Steinhardt <ps@pks.im> 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/xmqqzf4w8r20.fsf@gitster.g/); Junio’s
full-scan warning
(https://lore.kernel.org/git/xmqqfr6mt9uk.fsf@gitster.g/)
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/20260227063118.9069-1-github@paulisageek.com/)
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/aafifU-befdZW4O0@pks.im/); Paul’s
response (https://lore.kernel.org/git/20260304181745.25673-1-github@paulisageek.com/)
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-08-11 15:22 ` Tamir Duberstein
@ 2026-08-11 16:23 ` Patrick Steinhardt
2026-08-11 16:45 ` Tamir Duberstein
2026-08-11 17:35 ` Junio C Hamano
0 siblings, 2 replies; 9+ messages in thread
From: Patrick Steinhardt @ 2026-08-11 16:23 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Jeff Hostetler, Paul Tarjan, Junio C Hamano, Jeff King,
Taylor Blau, Johannes Schindelin, Koji Nakamaru
On Tue, Aug 11, 2026 at 11:22:01AM -0400, Tamir Duberstein wrote:
> On Wed, Aug 5, 2026 at 3:59 AM Patrick Steinhardt <ps@pks.im> 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.
>
[snip]
>
> Hope that's helpful.
Sorry, but that's not quite helpful. The questions I'm asking are to
verify whether you understand the consequences and subtleties around the
code area that you're proposing to change. If I wanted to only learn
about this myself then I could simply ask an agent myself, but that's
not really the intent of a code review.
So what I'm looking for is _your_ explanation, not the explanation of
AI. Your explanation may of course be informed by AI. But if so it's
your responsibility to double-check its assumptions, build your own
model and then share your informed opinion with us.
Right now I don't yet have the feeling that you understand why this
fixes the underlying issue.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-08-11 16:23 ` Patrick Steinhardt
@ 2026-08-11 16:45 ` Tamir Duberstein
2026-08-11 17:35 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Tamir Duberstein @ 2026-08-11 16:45 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Jeff Hostetler, Paul Tarjan, Junio C Hamano, Jeff King,
Taylor Blau, Johannes Schindelin, Koji Nakamaru
On Tue, Aug 11, 2026 at 12:23 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Aug 11, 2026 at 11:22:01AM -0400, Tamir Duberstein wrote:
> > On Wed, Aug 5, 2026 at 3:59 AM Patrick Steinhardt <ps@pks.im> 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.
> >
> [snip]
> >
> > Hope that's helpful.
>
> Sorry, but that's not quite helpful. The questions I'm asking are to
> verify whether you understand the consequences and subtleties around the
> code area that you're proposing to change. If I wanted to only learn
> about this myself then I could simply ask an agent myself, but that's
> not really the intent of a code review.
>
> So what I'm looking for is _your_ explanation, not the explanation of
> AI. Your explanation may of course be informed by AI. But if so it's
> your responsibility to double-check its assumptions, build your own
> model and then share your informed opinion with us.
>
> Right now I don't yet have the feeling that you understand why this
> fixes the underlying issue.
Got it. I agree with you that the flush call feels unnecessary under
the interpretation that passing 1ms to FSEventStreamCreate is the
equivalent of asking it to flush every 1ms. Empirically, though,
that's not the case, as described in the commit message.
There's more precedent for this technique (found by agent, sorry):
watchman fixed a similar issue here:
https://github.com/facebook/watchman/commit/d1795de4ecab33672a89802318fe6f0122462194
and the documented it here:
https://github.com/facebook/watchman/commit/2f80886991ce81585ac0679c2b019fa0e4d9e9dd
I agree this is unsatisfying.
Does that help?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fsmonitor: flush pending FSEvents before cookie wait
2026-08-11 16:23 ` Patrick Steinhardt
2026-08-11 16:45 ` Tamir Duberstein
@ 2026-08-11 17:35 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2026-08-11 17:35 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Tamir Duberstein, git, Jeff Hostetler, Paul Tarjan, Jeff King,
Taylor Blau, Johannes Schindelin, Koji Nakamaru
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Aug 11, 2026 at 11:22:01AM -0400, Tamir Duberstein wrote:
>...
>> Hope that's helpful.
>
> Sorry, but that's not quite helpful. The questions I'm asking are to
Thanks for pushing back.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-11 17:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 21:04 [PATCH] fsmonitor: flush pending FSEvents before cookie wait Tamir Duberstein
2026-07-24 2:41 ` Koji Nakamaru
2026-07-24 20:38 ` Junio C Hamano
2026-08-04 22:13 ` Junio C Hamano
2026-08-05 7:59 ` Patrick Steinhardt
2026-08-11 15:22 ` Tamir Duberstein
2026-08-11 16:23 ` Patrick Steinhardt
2026-08-11 16:45 ` Tamir Duberstein
2026-08-11 17:35 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.