From bc40a560d3c95040b55fd7be6fe5b7012d267f8f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Jun 2021 09:49:50 +0200 Subject: [PATCH 1/5] fixup! fsmonitor: introduce `core.useBuiltinFSMonitor` to call the daemon via IPC In FSMonitor v1, we made sure to only use a valid `since_token` when querying the FSMonitor. This condition was accidentally lost in v2, and caused segmentation faults uncovered by Scalar's Functional Tests. I had tried to fix this in https://github.com/git-for-windows/pull/3241, but the fix was incomplete, and I had to follow up with https://github.com/git-for-windows/pull/3258. However, it turns out that both of them were actually only work-arounds; I should have dug deeper to figure out _why_ the `since_token` was no longer guaranteed not to be `NULL`, and I finally did. Signed-off-by: Johannes Schindelin --- fsmonitor.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fsmonitor.c b/fsmonitor.c index 22623fd228fc61..0b40643442eb06 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -290,8 +290,9 @@ void refresh_fsmonitor(struct index_state *istate) trace_printf_key(&trace_fsmonitor, "refresh fsmonitor"); if (r->settings.use_builtin_fsmonitor > 0) { - query_success = !fsmonitor_ipc__send_query( - istate->fsmonitor_last_update, &query_result); + query_success = istate->fsmonitor_last_update && + !fsmonitor_ipc__send_query(istate->fsmonitor_last_update, + &query_result); if (query_success) { /* * The response contains a series of nul terminated From 7eb8372f9fdb6f331f1845f34f25df7aa36005e5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Jun 2021 09:54:50 +0200 Subject: [PATCH 2/5] fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon Now that we have a correct fix where we guarantee again (just like v1 of the built-in FSMonitor) that `since_token` is not `NULL`, we can revert the work-arounds introduced by these two PRs: - https://github.com/git-for-windows/pull/3241 - https://github.com/git-for-windows/pull/3258 Signed-off-by: Johannes Schindelin --- fsmonitor-ipc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c index 47afb4709f64e6..e62901a85b5dfe 100644 --- a/fsmonitor-ipc.c +++ b/fsmonitor-ipc.c @@ -44,7 +44,7 @@ int fsmonitor_ipc__send_query(const char *since_token, trace2_region_enter("fsm_client", "query", NULL); trace2_data_string("fsm_client", NULL, "query/command", - since_token ? since_token : "(null-token)"); + since_token); try_again: state = ipc_client_try_connect(fsmonitor_ipc__get_path(), &options, @@ -53,7 +53,7 @@ int fsmonitor_ipc__send_query(const char *since_token, switch (state) { case IPC_STATE__LISTENING: ret = ipc_client_send_command_to_connection( - connection, since_token, since_token ? strlen(since_token) : 0, answer); + connection, since_token, strlen(since_token), answer); ipc_client_close_connection(connection); trace2_data_intmax("fsm_client", NULL, From 5ec07b8c4a8ce7f4a9f3a8a74d87545955947e02 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Jun 2021 09:59:58 +0200 Subject: [PATCH 3/5] fixup! fsmonitor--daemon: use a cookie file to sync with file system When flushing the FSMonitor data, we do not actually need to wait for a cookie file. In the worst case, we will over-report a bit. If we _do_ wait for a cookie file, in the worst case we cause a hang because the FSMonitor daemon will wait and wait even though the `.git/` directory might be gone already. Signed-off-by: Johannes Schindelin --- builtin/fsmonitor--daemon.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 985a82cf39e076..0f76cdb1a2ca70 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -672,7 +672,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, */ do_flush = 1; do_trivial = 1; - do_cookie = 1; } else if (!skip_prefix(command, "builtin:", &p)) { /* assume V1 timestamp or garbage */ From 92392daccb31c405c529a772fc8b9ca63631e630 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Jun 2021 09:59:58 +0200 Subject: [PATCH 4/5] fixup! fsmonitor--daemon: use a cookie file to sync with file system When the built-in FSMonitor receives an unexpected token, we do not actually need to wait for a cookie file. There simply is no use for waiting in this instance. It's not like the client will all of a sudden realize that it sent an incorrect token and somehow make up a correct token from thin air in a follow-up query. Signed-off-by: Johannes Schindelin --- builtin/fsmonitor--daemon.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 0f76cdb1a2ca70..9e0a12024a7c8c 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -685,7 +685,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, "fsmonitor: unsupported V1 protocol '%s'"), command); do_trivial = 1; - do_cookie = 1; } else { /* We have "builtin:*" */ From cfa0c45308ef349b6f0a4ffa9e2ccd8d026a9f4b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 9 Jun 2021 09:59:58 +0200 Subject: [PATCH 5/5] fixup! fsmonitor--daemon: use a cookie file to sync with file system When the built-in FSMonitor receives an invalid v2 token, we do not actually need to wait for a cookie file. There simply is no use for waiting in this instance. It's not like the client will all of a sudden realize that it sent an incorrect token and somehow make up a correct token from thin air in a follow-up query. Signed-off-by: Johannes Schindelin --- builtin/fsmonitor--daemon.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 9e0a12024a7c8c..4afbb36fe61cae 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -694,7 +694,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, "fsmonitor: invalid V2 protocol token '%s'", command); do_trivial = 1; - do_cookie = 1; } else { /*