diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index 037a91cbcaf8fa..fd6012acba1311 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -599,6 +599,46 @@ stopping after the waitpid() and includes OS process creation overhead). So this time will be slightly larger than the atexit time reported by the child process itself. +`"child_ready"`:: + This event is generated after the current process has started + a background process and released all handles to it. ++ +------------ +{ + "event":"child_ready", + ... + "child_id":2, + "pid":14708, # child PID + "ready":"ready", # child ready state + "t_rel":0.110605 # observed run-time of child process +} +------------ ++ +Note that the session-id of the child process is not available to +the current/spawning process, so the child's PID is reported here as +a hint for post-processing. (But it is only a hint because the child +process may be a shell script which doesn't have a session-id.) ++ +This event is generated after the child is started in the background +and given a little time to boot up and start working. If the child +startups normally and while the parent is still waiting, the "ready" +field will have the value "ready". +If the child is too slow to start and the parent times out, the field +will have the value "timeout". +If the child starts but the parent is unable to probe it, the field +will have the value "error". ++ +After the parent process emits this event, it will release all of its +handles to the child process and treat the child as a background +daemon. So even if the child does eventually finish booting up, +the parent will not emit an updated event. ++ +Note that the `t_rel` field contains the observed run time in seconds +when the parent released the child process into the background. +The child is assumed to be a long-running daemon process and may +outlive the parent process. So the parent's child event times should +not be compared to the child's atexit times. + `"exec"`:: This event is generated before git attempts to `exec()` another command rather than starting a child process. diff --git a/Makefile b/Makefile index 4262a7dcf1fabc..536303b7d643f2 100644 --- a/Makefile +++ b/Makefile @@ -473,8 +473,9 @@ all:: # # If your platform supports a built-in fsmonitor backend, set # FSMONITOR_DAEMON_BACKEND to the "" of the corresponding -# `compat/fsmonitor/fsm-listen-.c` that implements the -# `fsm_listen__*()` routines. +# `compat/fsmonitor/fsm-listen-.c` and +# `compat/fsmonitor/fsm-health-.c` files +# that implement the `fsm_listen__*()` and `fsm_health__*()` routines. # # If your platform has os-specific ways to tell if a repo is incompatible with # fsmonitor (whether the hook or ipc daemon version), set FSMONITOR_OS_SETTINGS @@ -1963,6 +1964,7 @@ endif ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGS diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index d9a9b8e6b832ad..591433e897df15 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -3,6 +3,7 @@ #include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h" #include "simple-ipc.h" @@ -27,6 +28,9 @@ static int fsmonitor__ipc_threads = 8; #define FSMONITOR__START_TIMEOUT "fsmonitor.starttimeout" static int fsmonitor__start_timeout_sec = 60; +#define FSMONITOR__ANNOUNCE_STARTUP "fsmonitor.announcestartup" +static int fsmonitor__announce_startup = 0; + static int fsmonitor_config(const char *var, const char *value, void *cb) { if (!strcmp(var, FSMONITOR__IPC_THREADS)) { @@ -47,6 +51,16 @@ static int fsmonitor_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, FSMONITOR__ANNOUNCE_STARTUP)) { + int is_bool; + int i = git_config_bool_or_int(var, value, &is_bool); + if (i < 0) + return error(_("value of '%s' not bool or int: %d"), + var, i); + fsmonitor__announce_startup = i; + return 0; + } + return git_default_config(var, value, cb); } @@ -400,22 +414,21 @@ struct fsmonitor_batch *fsmonitor_batch__new(void) return batch; } -struct fsmonitor_batch *fsmonitor_batch__pop(struct fsmonitor_batch *batch) +void fsmonitor_batch__free_list(struct fsmonitor_batch *batch) { - struct fsmonitor_batch *next; - - if (!batch) - return NULL; - - next = batch->next; + while (batch) { + struct fsmonitor_batch *next = batch->next; - /* - * The actual strings within the array are interned, so we don't - * own them. - */ - free(batch->interned_paths); + /* + * The actual strings within the array of this batch + * are interned, so we don't own them. We only own + * the array. + */ + free(batch->interned_paths); + free(batch); - return next; + batch = next; + } } void fsmonitor_batch__add_path(struct fsmonitor_batch *batch, @@ -512,24 +525,8 @@ static struct fsmonitor_batch *with_lock__truncate_old_batches( return remainder; } -static void free_remainder(struct fsmonitor_batch *remainder) -{ - struct fsmonitor_batch *p; - - if (!remainder) - return; - - for (p = remainder; p; p = fsmonitor_batch__pop(p)) { - trace_printf_key(&trace_fsmonitor, - "Truncate: kill (%"PRIu64",%"PRIu64")", - p->batch_seq_nr, (uint64_t)p->pinned_time); - } -} - static void fsmonitor_free_token_data(struct fsmonitor_token_data *token) { - struct fsmonitor_batch *p; - if (!token) return; @@ -537,8 +534,7 @@ static void fsmonitor_free_token_data(struct fsmonitor_token_data *token) strbuf_release(&token->token_id); - for (p = token->batch_head; p; p = fsmonitor_batch__pop(p)) - ; + fsmonitor_batch__free_list(token->batch_head); free(token); } @@ -631,7 +627,7 @@ static int fsmonitor_parse_client_token(const char *buf_token, return 0; } -KHASH_INIT(str, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal); +KHASH_INIT(str, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal) static int do_handle_client(struct fsmonitor_daemon_state *state, const char *command, @@ -906,19 +902,26 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, fsmonitor_free_token_data(token_data); } else if (batch) { /* - * This batch is the first item in the list - * that is older than the requested sequence - * number and might be considered to be - * obsolete. See if we can truncate the list - * and save some memory. + * We are holding the lock and are the only + * reader of the ref-counted portion of the + * list, so we get the honor of seeing if the + * list can be truncated to save memory. + * + * The main loop did not walk to the end of the + * list, so this batch is the first item in the + * batch-list that is older than the requested + * end-point sequence number. See if the tail + * end of the list is obsolete. */ - remainder = with_lock__truncate_old_batches(state, batch); + remainder = with_lock__truncate_old_batches(state, + batch); } } pthread_mutex_unlock(&state->main_lock); - free_remainder(remainder); + if (remainder) + fsmonitor_batch__free_list(remainder); trace2_data_intmax("fsmonitor", the_repository, "response/length", total_response_len); trace2_data_intmax("fsmonitor", the_repository, "response/count/files", count); @@ -1094,7 +1097,7 @@ void fsmonitor_publish(struct fsmonitor_daemon_state *state, * `fsmonitor_new_token_data()` about why we * don't need to accumulate these paths. */ - fsmonitor_batch__pop(batch); + fsmonitor_batch__free_list(batch); } else if (head->nr + batch->nr > MY_COMBINE_LIMIT) { /* * The head batch in the list has never been @@ -1108,11 +1111,11 @@ void fsmonitor_publish(struct fsmonitor_daemon_state *state, state->current_token_data->batch_head = batch; } else { /* - * We are free to append the paths in the given + * We are free to add the paths in the given * batch onto the end of the current head batch. */ fsmonitor_batch__combine(head, batch); - fsmonitor_batch__pop(batch); + fsmonitor_batch__free_list(batch); } } @@ -1122,6 +1125,18 @@ void fsmonitor_publish(struct fsmonitor_daemon_state *state, pthread_mutex_unlock(&state->main_lock); } +static void *fsm_health__thread_proc(void *_state) +{ + struct fsmonitor_daemon_state *state = _state; + + trace2_thread_start("fsm-health"); + + fsm_health__loop(state); + + trace2_thread_exit(); + return NULL; +} + static void *fsm_listen__thread_proc(void *_state) { struct fsmonitor_daemon_state *state = _state; @@ -1160,6 +1175,9 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) */ .uds_disallow_chdir = 0 }; + int health_started = 0; + int listener_started = 0; + int err = 0; /* * Start the IPC thread pool before the we've started the file @@ -1167,9 +1185,11 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) * before we need it. */ if (ipc_server_run_async(&state->ipc_server_data, - fsmonitor_ipc__get_path(), &ipc_opts, + state->path_ipc.buf, &ipc_opts, handle_client, state)) - return error(_("could not start IPC thread pool")); + return error_errno( + _("could not start IPC thread pool on '%s'"), + state->path_ipc.buf); /* * Start the fsmonitor listener thread to collect filesystem @@ -1178,15 +1198,31 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) if (pthread_create(&state->listener_thread, NULL, fsm_listen__thread_proc, state) < 0) { ipc_server_stop_async(state->ipc_server_data); - ipc_server_await(state->ipc_server_data); + err = error(_("could not start fsmonitor listener thread")); + goto cleanup; + } + listener_started = 1; - return error(_("could not start fsmonitor listener thread")); + /* + * Start the health thread to watch over our process. + */ + if (pthread_create(&state->health_thread, NULL, + fsm_health__thread_proc, state) < 0) { + ipc_server_stop_async(state->ipc_server_data); + err = error(_("could not start fsmonitor health thread")); + goto cleanup; } + health_started = 1; /* * The daemon is now fully functional in background threads. + * Our primary thread should now just wait while the threads + * do all the work. + */ +cleanup: + /* * Wait for the IPC thread pool to shutdown (whether by client - * request or from filesystem activity). + * request, from filesystem activity, or an error). */ ipc_server_await(state->ipc_server_data); @@ -1195,15 +1231,29 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) * event from the IPC thread pool, but it doesn't hurt to tell * it again. And wait for it to shutdown. */ - fsm_listen__stop_async(state); - pthread_join(state->listener_thread, NULL); + if (listener_started) { + fsm_listen__stop_async(state); + pthread_join(state->listener_thread, NULL); + } + + if (health_started) { + fsm_health__stop_async(state); + pthread_join(state->health_thread, NULL); + } - return state->error_code; + if (err) + return err; + if (state->listen_error_code) + return state->listen_error_code; + if (state->health_error_code) + return state->health_error_code; + return 0; } static int fsmonitor_run_daemon(void) { struct fsmonitor_daemon_state state; + const char *home; int err; memset(&state, 0, sizeof(state)); @@ -1211,7 +1261,8 @@ static int fsmonitor_run_daemon(void) hashmap_init(&state.cookies, cookies_cmp, NULL, 0); pthread_mutex_init(&state.main_lock, NULL); pthread_cond_init(&state.cookies_cond, NULL); - state.error_code = 0; + state.listen_error_code = 0; + state.health_error_code = 0; state.current_token_data = fsmonitor_new_token_data(); /* Prepare to (recursively) watch the directory. */ @@ -1273,6 +1324,15 @@ static int fsmonitor_run_daemon(void) strbuf_addch(&state.path_cookie_prefix, '/'); + /* + * We create a named-pipe or unix domain socket inside of the + * ".git" directory. (Well, on Windows, we base our named + * pipe in the NPFS on the absolute path of the git + * directory.) + */ + strbuf_init(&state.path_ipc, 0); + strbuf_addstr(&state.path_ipc, absolute_path(fsmonitor_ipc__get_path())); + /* * Confirm that we can create platform-specific resources for the * filesystem listener before we bother starting all the threads. @@ -1282,18 +1342,42 @@ static int fsmonitor_run_daemon(void) goto done; } + if (fsm_health__ctor(&state)) { + err = error(_("could not initialize health thread")); + goto done; + } + + /* + * CD out of the worktree root directory. + * + * The common Git startup mechanism causes our CWD to be the + * root of the worktree. On Windows, this causes our process + * to hold a locked handle on the CWD. This prevents the + * worktree from being moved or deleted while the daemon is + * running. + * + * We assume that our FS and IPC listener threads have either + * opened all of the handles that they need or will do + * everything using absolute paths. + */ + home = getenv("HOME"); + if (home && *home && chdir(home)) + die_errno("could not cd home '%s'", home); + err = fsmonitor_run_daemon_1(&state); done: pthread_cond_destroy(&state.cookies_cond); pthread_mutex_destroy(&state.main_lock); fsm_listen__dtor(&state); + fsm_health__dtor(&state); ipc_server_free(state.ipc_server_data); strbuf_release(&state.path_worktree_watch); strbuf_release(&state.path_gitdir_watch); strbuf_release(&state.path_cookie_prefix); + strbuf_release(&state.path_ipc); /* * NEEDSWORK: Consider "rm -rf /" @@ -1316,9 +1400,11 @@ static int try_to_run_foreground_daemon(int free_console) die("fsmonitor--daemon is already running '%s'", the_repository->worktree); - printf(_("running fsmonitor-daemon in '%s'\n"), - the_repository->worktree); - fflush(stdout); + if (fsmonitor__announce_startup) { + fprintf(stderr, _("running fsmonitor-daemon in '%s'\n"), + the_repository->worktree); + fflush(stderr); + } #ifdef GIT_WINDOWS_NATIVE if (free_console) @@ -1328,160 +1414,34 @@ static int try_to_run_foreground_daemon(int free_console) return !!fsmonitor_run_daemon(); } -#ifdef GIT_WINDOWS_NATIVE -/* - * Create a background process to run the daemon. It should be completely - * disassociated from the terminal. - * - * Conceptually like `daemonize()` but different because Windows does not - * have `fork(2)`. Spawn a normal Windows child process but without the - * limitations of `start_command()` and `finish_command()`. - * - * The child process execs the "git fsmonitor--daemon run" command. - * - * The current process returns so that the caller can wait for the child - * to startup before exiting. - */ -static int spawn_fsmonitor(pid_t *pid) -{ - char git_exe[MAX_PATH]; - struct strvec args = STRVEC_INIT; - int in, out; - - GetModuleFileNameA(NULL, git_exe, MAX_PATH); - - in = open("/dev/null", O_RDONLY); - out = open("/dev/null", O_WRONLY); - - strvec_push(&args, git_exe); - strvec_push(&args, "fsmonitor--daemon"); - strvec_push(&args, "run"); - strvec_push(&args, "--free-console"); - strvec_pushf(&args, "--ipc-threads=%d", fsmonitor__ipc_threads); +static start_bg_wait_cb bg_wait_cb; - *pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out); - close(in); - close(out); - - strvec_clear(&args); - - if (*pid < 0) - return error(_("could not spawn fsmonitor--daemon in the background")); - - return 0; -} -#else -/* - * Create a background process to run the daemon. It should be completely - * disassociated from the terminal. - * - * This is adapted from `daemonize()`. Use `fork()` to directly - * create and run the daemon in the child process. - * - * The fork-child can just call the run code; it does not need to exec - * it. - * - * The fork-parent returns the child PID so that we can wait for the - * child to startup before exiting. - */ -static int spawn_fsmonitor(pid_t *pid) +static int bg_wait_cb(const struct child_process *cp, void *cb_data) { - *pid = fork(); + enum ipc_active_state s = fsmonitor_ipc__get_state(); - switch (*pid) { - case 0: - if (setsid() == -1) - error_errno(_("setsid failed")); - close(0); - close(1); - close(2); - sanitize_stdfds(); - - return !!fsmonitor_run_daemon(); - - case -1: - return error_errno(_("could not spawn fsmonitor--daemon in the background")); - - default: + switch (s) { + case IPC_STATE__LISTENING: + /* child is "ready" */ return 0; - } -} -#endif - -/* - * This is adapted from `wait_or_whine()`. Watch the child process and - * let it get started and begin listening for requests on the socket - * before reporting our success. - */ -static int wait_for_startup(pid_t pid_child) -{ - int status; - pid_t pid_seen; - enum ipc_active_state s; - time_t time_limit, now; - time(&time_limit); - time_limit += fsmonitor__start_timeout_sec; - - for (;;) { - pid_seen = waitpid(pid_child, &status, WNOHANG); - - if (pid_seen == -1) - return error_errno(_("waitpid failed")); - else if (pid_seen == 0) { - /* - * The child is still running (this should be - * the normal case). Try to connect to it on - * the socket and see if it is ready for - * business. - * - * If there is another daemon already running, - * our child will fail to start (possibly - * after a timeout on the lock), but we don't - * care (who responds) if the socket is live. - */ - s = fsmonitor_ipc__get_state(); - if (s == IPC_STATE__LISTENING) - return 0; - - time(&now); - if (now > time_limit) - return error(_("fsmonitor--daemon not online yet")); - } else if (pid_seen == pid_child) { - /* - * The new child daemon process shutdown while - * it was starting up, so it is not listening - * on the socket. - * - * Try to ping the socket in the odd chance - * that another daemon started (or was already - * running) while our child was starting. - * - * Again, we don't care who services the socket. - */ - s = fsmonitor_ipc__get_state(); - if (s == IPC_STATE__LISTENING) - return 0; + case IPC_STATE__NOT_LISTENING: + case IPC_STATE__PATH_NOT_FOUND: + /* give child more time */ + return 1; - /* - * We don't care about the WEXITSTATUS() nor - * any of the WIF*(status) values because - * `cmd_fsmonitor__daemon()` does the `!!result` - * trick on all function return values. - * - * So it is sufficient to just report the - * early shutdown as an error. - */ - return error(_("fsmonitor--daemon failed to start")); - } else - return error(_("waitpid is confused")); + default: + case IPC_STATE__INVALID_PATH: + case IPC_STATE__OTHER_ERROR: + /* all the time in world won't help */ + return -1; } } static int try_to_start_background_daemon(void) { - pid_t pid_child; - int ret; + struct child_process cp = CHILD_PROCESS_INIT; + enum start_bg_result sbgr; /* * Before we try to create a background daemon process, see @@ -1495,26 +1455,41 @@ static int try_to_start_background_daemon(void) die("fsmonitor--daemon is already running '%s'", the_repository->worktree); - printf(_("starting fsmonitor-daemon in '%s'\n"), - the_repository->worktree); - fflush(stdout); + if (fsmonitor__announce_startup) { + fprintf(stderr, _("starting fsmonitor-daemon in '%s'\n"), + the_repository->worktree); + fflush(stderr); + } - /* - * Run the actual daemon in a background process. - */ - ret = spawn_fsmonitor(&pid_child); - if (pid_child <= 0) - return ret; + cp.git_cmd = 1; - /* - * Wait (with timeout) for the background child process get - * started and begin listening on the socket/pipe. This makes - * the "start" command more synchronous and more reliable in - * tests. - */ - ret = wait_for_startup(pid_child); + strvec_push(&cp.args, "fsmonitor--daemon"); + strvec_push(&cp.args, "run"); + strvec_push(&cp.args, "--free-console"); + strvec_pushf(&cp.args, "--ipc-threads=%d", fsmonitor__ipc_threads); + + cp.no_stdin = 1; + cp.no_stdout = 1; + cp.no_stderr = 1; + + sbgr = start_bg_command(&cp, bg_wait_cb, NULL, + fsmonitor__start_timeout_sec); - return ret; + switch (sbgr) { + case SBGR_READY: + return 0; + + default: + case SBGR_ERROR: + case SBGR_CB_ERROR: + return error("daemon failed to start"); + + case SBGR_TIMEOUT: + return error("daemon not online yet"); + + case SBGR_DIED: + return error("daemon terminated"); + } } int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix) @@ -1534,27 +1509,21 @@ int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix) OPT_END() }; - if (argc < 2) - usage_with_options(builtin_fsmonitor__daemon_usage, options); - - if (argc == 2 && !strcmp(argv[1], "-h")) - usage_with_options(builtin_fsmonitor__daemon_usage, options); - git_config(fsmonitor_config, NULL); - subcmd = argv[1]; - argv--; - argc++; - argc = parse_options(argc, argv, prefix, options, builtin_fsmonitor__daemon_usage, 0); + if (argc != 1) + usage_with_options(builtin_fsmonitor__daemon_usage, options); + subcmd = argv[0]; + if (fsmonitor__ipc_threads < 1) die(_("invalid 'ipc-threads' value (%d)"), fsmonitor__ipc_threads); prepare_repo_settings(the_repository); - fsm_settings__set_ipc(the_repository); + if (fsm_settings__get_mode(the_repository) == FSMONITOR_MODE_INCOMPATIBLE) { struct strbuf buf_reason = STRBUF_INIT; fsm_settings__get_reason(the_repository, &buf_reason); diff --git a/compat/fsmonitor/fsm-health-darwin.c b/compat/fsmonitor/fsm-health-darwin.c new file mode 100644 index 00000000000000..b9f709e854833f --- /dev/null +++ b/compat/fsmonitor/fsm-health-darwin.c @@ -0,0 +1,24 @@ +#include "cache.h" +#include "config.h" +#include "fsmonitor.h" +#include "fsm-health.h" +#include "fsmonitor--daemon.h" + +int fsm_health__ctor(struct fsmonitor_daemon_state *state) +{ + return 0; +} + +void fsm_health__dtor(struct fsmonitor_daemon_state *state) +{ + return; +} + +void fsm_health__loop(struct fsmonitor_daemon_state *state) +{ + return; +} + +void fsm_health__stop_async(struct fsmonitor_daemon_state *state) +{ +} diff --git a/compat/fsmonitor/fsm-health-win32.c b/compat/fsmonitor/fsm-health-win32.c new file mode 100644 index 00000000000000..2c3d7e4b2ae79a --- /dev/null +++ b/compat/fsmonitor/fsm-health-win32.c @@ -0,0 +1,257 @@ +#include "cache.h" +#include "config.h" +#include "fsmonitor.h" +#include "fsm-health.h" +#include "fsmonitor--daemon.h" + +/* + * Every minute wake up and test our health. + */ +#define WAIT_FREQ_MS (60 * 1000) + +enum interval_fn_ctx { CTX_INIT = 0, CTX_TERM, CTX_TIMER }; + +typedef int (interval_fn)(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx); + +static interval_fn has_worktree_moved; + +static interval_fn *table[] = { + has_worktree_moved, + NULL, /* must be last */ +}; + +/* + * Call all of the functions in the table. + * Shortcut and return first error. + * + * Return 0 if all succeeded. + */ +static int call_all(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx) +{ + int k; + + for (k = 0; table[k]; k++) { + int r = table[k](state, ctx); + if (r) + return r; + } + + return 0; +} + +struct fsm_health_data +{ + HANDLE hEventShutdown; + + HANDLE hHandles[1]; /* the array does not own these handles */ +#define HEALTH_SHUTDOWN 0 + int nr_handles; /* number of active event handles */ + + struct wt_moved + { + wchar_t wpath[MAX_LONG_PATH + 1]; + BY_HANDLE_FILE_INFORMATION bhfi; + } wt_moved; +}; + +int fsm_health__ctor(struct fsmonitor_daemon_state *state) +{ + struct fsm_health_data *data; + + CALLOC_ARRAY(data, 1); + + data->hEventShutdown = CreateEvent(NULL, TRUE, FALSE, NULL); + + data->hHandles[HEALTH_SHUTDOWN] = data->hEventShutdown; + data->nr_handles++; + + state->health_data = data; + return 0; +} + +void fsm_health__dtor(struct fsmonitor_daemon_state *state) +{ + struct fsm_health_data *data; + + if (!state || !state->health_data) + return; + + data = state->health_data; + + CloseHandle(data->hEventShutdown); + + FREE_AND_NULL(state->health_data); +} + +static int lookup_bhfi(wchar_t *wpath, + BY_HANDLE_FILE_INFORMATION *bhfi) +{ + DWORD desired_access = FILE_LIST_DIRECTORY; + DWORD share_mode = + FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; + HANDLE hDir; + + hDir = CreateFileW(wpath, desired_access, share_mode, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hDir == INVALID_HANDLE_VALUE) { + error(_("[GLE %ld] health thread could not open '%ls'"), + GetLastError(), wpath); + return -1; + } + + if (!GetFileInformationByHandle(hDir, bhfi)) { + error(_("[GLE %ld] health thread getting BHFI for '%ls'"), + GetLastError(), wpath); + CloseHandle(hDir); + return -1; + } + + CloseHandle(hDir); + return 0; +} + +static int bhfi_eq(const BY_HANDLE_FILE_INFORMATION *bhfi_1, + const BY_HANDLE_FILE_INFORMATION *bhfi_2) +{ + return (bhfi_1->dwVolumeSerialNumber == bhfi_2->dwVolumeSerialNumber && + bhfi_1->nFileIndexHigh == bhfi_2->nFileIndexHigh && + bhfi_1->nFileIndexLow == bhfi_2->nFileIndexLow); +} + +/* + * Shutdown if the original worktree root directory been deleted, + * moved, or renamed? + * + * Since the main thread did a "chdir(getenv($HOME))" and our CWD + * is not in the worktree root directory and because the listener + * thread added FILE_SHARE_DELETE to the watch handle, it is possible + * for the root directory to be moved or deleted while we are still + * watching it. We want to detect that here and force a shutdown. + * + * Granted, a delete MAY cause some operations to fail, such as + * GetOverlappedResult(), but it is not guaranteed. And because + * ReadDirectoryChangesW() only reports on changes *WITHIN* the + * directory, not changes *ON* the directory, our watch will not + * receive a delete event for it. + * + * A move/rename of the worktree root will also not generate an event. + * And since the listener thread already has an open handle, it may + * continue to receive events for events within the directory. + * However, the pathname of the named-pipe was constructed using the + * original location of the worktree root. (Remember named-pipes are + * stored in the NPFS and not in the actual file system.) Clients + * trying to talk to the worktree after the move/rename will not + * reach our daemon process, since we're still listening on the + * pipe with original path. + * + * Furthermore, if the user does something like: + * + * $ mv repo repo.old + * $ git init repo + * + * A new daemon cannot be started in the new instance of "repo" + * because the named-pipe is still being used by the daemon on + * the original instance. + * + * So, detect move/rename/delete and shutdown. This should also + * handle unsafe drive removal. + * + * We use the file system unique ID to distinguish the original + * directory instance from a new instance and force a shutdown + * if the unique ID changes. + * + * Since a worktree move/rename/delete/unmount doesn't happen + * that often (and we can't get an immediate event anyway), we + * use a timeout and periodically poll it. + */ +static int has_worktree_moved(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx) +{ + struct fsm_health_data *data = state->health_data; + BY_HANDLE_FILE_INFORMATION bhfi; + int r; + + switch (ctx) { + case CTX_TERM: + return 0; + + case CTX_INIT: + if (xutftowcs_long_path(data->wt_moved.wpath, + state->path_worktree_watch.buf) < 0) { + error(_("could not convert to wide characters: '%s'"), + state->path_worktree_watch.buf); + return -1; + } + + /* + * On the first call we lookup the unique sequence ID for + * the worktree root directory. + */ + return lookup_bhfi(data->wt_moved.wpath, &data->wt_moved.bhfi); + + case CTX_TIMER: + r = lookup_bhfi(data->wt_moved.wpath, &bhfi); + if (r) + return r; + if (!bhfi_eq(&data->wt_moved.bhfi, &bhfi)) { + error(_("BHFI changed '%ls'"), data->wt_moved.wpath); + return -1; + } + return 0; + + default: + die("unhandled case in 'has_worktree_moved': %d", + (int)ctx); + } + + return 0; +} + +void fsm_health__loop(struct fsmonitor_daemon_state *state) +{ + struct fsm_health_data *data = state->health_data; + int r; + + r = call_all(state, CTX_INIT); + if (r < 0) + goto force_error_stop; + if (r > 0) + goto force_shutdown; + + for (;;) { + DWORD dwWait = WaitForMultipleObjects(data->nr_handles, + data->hHandles, + FALSE, WAIT_FREQ_MS); + + if (dwWait == WAIT_OBJECT_0 + HEALTH_SHUTDOWN) + goto clean_shutdown; + + if (dwWait == WAIT_TIMEOUT) { + r = call_all(state, CTX_TIMER); + if (r < 0) + goto force_error_stop; + if (r > 0) + goto force_shutdown; + continue; + } + + error(_("health thread wait failed [GLE %ld]"), + GetLastError()); + goto force_error_stop; + } + +force_error_stop: + state->health_error_code = -1; +force_shutdown: + ipc_server_stop_async(state->ipc_server_data); +clean_shutdown: + call_all(state, CTX_TERM); + return; +} + +void fsm_health__stop_async(struct fsmonitor_daemon_state *state) +{ + SetEvent(state->health_data->hHandles[HEALTH_SHUTDOWN]); +} diff --git a/compat/fsmonitor/fsm-health.h b/compat/fsmonitor/fsm-health.h new file mode 100644 index 00000000000000..45547ba93804a9 --- /dev/null +++ b/compat/fsmonitor/fsm-health.h @@ -0,0 +1,47 @@ +#ifndef FSM_HEALTH_H +#define FSM_HEALTH_H + +/* This needs to be implemented by each backend */ + +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND + +struct fsmonitor_daemon_state; + +/* + * Initialize platform-specific data for the fsmonitor health thread. + * This will be called from the main thread PRIOR to staring the + * thread. + * + * Returns 0 if successful. + * Returns -1 otherwise. + */ +int fsm_health__ctor(struct fsmonitor_daemon_state *state); + +/* + * Cleanup platform-specific data for the health thread. + * This will be called from the main thread AFTER joining the thread. + */ +void fsm_health__dtor(struct fsmonitor_daemon_state *state); + +/* + * The main body of the platform-specific event loop to monitor the + * health of the daemon process. This will run in the health thread. + * + * The health thread should call `ipc_server_stop_async()` if it needs + * to cause a shutdown. (It should NOT do so if it receives a shutdown + * shutdown signal.) + * + * It should set `state->health_error_code` to -1 if the daemon should exit + * with an error. + */ +void fsm_health__loop(struct fsmonitor_daemon_state *state); + +/* + * Gently request that the health thread shutdown. + * It does not wait for it to stop. The caller should do a JOIN + * to wait for it. + */ +void fsm_health__stop_async(struct fsmonitor_daemon_state *state); + +#endif /* HAVE_FSMONITOR_DAEMON_BACKEND */ +#endif /* FSM_HEALTH_H */ diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c index 83ae3609c1b44e..d3afbbc53d68bb 100644 --- a/compat/fsmonitor/fsm-listen-darwin.c +++ b/compat/fsmonitor/fsm-listen-darwin.c @@ -1,4 +1,4 @@ -#if defined(__GNUC__) +#ifndef __clang__ /* * It is possible to #include CoreFoundation/CoreFoundation.h when compiling * with clang, but not with GCC as of time of writing. @@ -99,7 +99,7 @@ void FSEventStreamRelease(FSEventStreamRef stream); #include "fsm-listen.h" #include "fsmonitor--daemon.h" -struct fsmonitor_daemon_backend_data +struct fsm_listen_data { CFStringRef cfsr_worktree_path; CFStringRef cfsr_gitdir_path; @@ -172,12 +172,17 @@ static void log_flags_set(const char *path, const FSEventStreamEventFlags flag) if (flag & kFSEventStreamEventFlagItemCloned) strbuf_addstr(&msg, "ItemCloned|"); - trace_printf_key(&trace_fsmonitor, "fsevent: '%s', flags=%u %s", + trace_printf_key(&trace_fsmonitor, "fsevent: '%s', flags=0x%x %s", path, flag, msg.buf); strbuf_release(&msg); } +static int ef_is_root_changed(const FSEventStreamEventFlags ef) +{ + return (ef & kFSEventStreamEventFlagRootChanged); +} + static int ef_is_root_delete(const FSEventStreamEventFlags ef) { return (ef & kFSEventStreamEventFlagItemIsDir && @@ -192,10 +197,36 @@ static int ef_is_root_renamed(const FSEventStreamEventFlags ef) static int ef_is_dropped(const FSEventStreamEventFlags ef) { - return (ef & kFSEventStreamEventFlagKernelDropped || + return (ef & kFSEventStreamEventFlagMustScanSubDirs || + ef & kFSEventStreamEventFlagKernelDropped || ef & kFSEventStreamEventFlagUserDropped); } +/* + * If an `xattr` change is the only reason we received this event, + * then silently ignore it. Git doesn't care about xattr's. We + * have to be careful here because the kernel can combine multiple + * events for a single path. And because events always have certain + * bits set, such as `ItemIsFile` or `ItemIsDir`. + * + * Return 1 if we should ignore it. + */ +static int ef_ignore_xattr(const FSEventStreamEventFlags ef) +{ + static const FSEventStreamEventFlags mask = + kFSEventStreamEventFlagItemChangeOwner | + kFSEventStreamEventFlagItemCreated | + kFSEventStreamEventFlagItemFinderInfoMod | + kFSEventStreamEventFlagItemInodeMetaMod | + kFSEventStreamEventFlagItemModified | + kFSEventStreamEventFlagItemRemoved | + kFSEventStreamEventFlagItemRenamed | + kFSEventStreamEventFlagItemXattrMod | + kFSEventStreamEventFlagItemCloned; + + return ((ef & mask) == kFSEventStreamEventFlagItemXattrMod); +} + static void fsevent_callback(ConstFSEventStreamRef streamRef, void *ctx, size_t num_of_events, @@ -204,7 +235,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, const FSEventStreamEventId event_ids[]) { struct fsmonitor_daemon_state *state = ctx; - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; char **paths = (char **)event_paths; struct fsmonitor_batch *batch = NULL; struct string_list cookie_list = STRING_LIST_INIT_DUP; @@ -244,13 +275,11 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, * token). */ if (ef_is_dropped(event_flags[k])) { - /* - * see also kFSEventStreamEventFlagMustScanSubDirs - */ - trace_printf_key(&trace_fsmonitor, "event: dropped"); + if (trace_pass_fl(&trace_fsmonitor)) + log_flags_set(path_k, event_flags[k]); fsmonitor_force_resync(state); - fsmonitor_batch__pop(batch); + fsmonitor_batch__free_list(batch); string_list_clear(&cookie_list, 0); /* @@ -263,6 +292,33 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, continue; } + if (ef_is_root_changed(event_flags[k])) { + /* + * The spelling of the pathname of the root directory + * has changed. This includes the name of the root + * directory itself of of any parent directory in the + * path. + * + * (There may be other conditions that throw this, + * but I couldn't find any information on it.) + * + * Force a shutdown now and avoid things getting + * out of sync. The Unix domain socket is inside + * the .git directory and a spelling change will make + * it hard for clients to rendezvous with us. + */ + trace_printf_key(&trace_fsmonitor, + "event: root changed"); + goto force_shutdown; + } + + if (ef_ignore_xattr(event_flags[k])) { + trace_printf_key(&trace_fsmonitor, + "ignore-xattr: '%s', flags=0x%x", + path_k, event_flags[k]); + continue; + } + switch (fsmonitor_classify_path_absolute(state, path_k)) { case IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX: @@ -353,7 +409,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, return; force_shutdown: - fsmonitor_batch__pop(batch); + fsmonitor_batch__free_list(batch); string_list_clear(&cookie_list, 0); data->shutdown_style = FORCE_SHUTDOWN; @@ -388,11 +444,11 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) NULL, NULL }; - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; const void *dir_array[2]; CALLOC_ARRAY(data, 1); - state->backend_data = data; + state->listen_data = data; data->cfsr_worktree_path = CFStringCreateWithCString( NULL, state->path_worktree_watch.buf, kCFStringEncodingUTF8); @@ -424,18 +480,18 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) failed: error("Unable to create FSEventStream."); - FREE_AND_NULL(state->backend_data); + FREE_AND_NULL(state->listen_data); return -1; } void fsm_listen__dtor(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - if (!state || !state->backend_data) + if (!state || !state->listen_data) return; - data = state->backend_data; + data = state->listen_data; if (data->stream) { if (data->stream_started) @@ -445,14 +501,14 @@ void fsm_listen__dtor(struct fsmonitor_daemon_state *state) FSEventStreamRelease(data->stream); } - FREE_AND_NULL(state->backend_data); + FREE_AND_NULL(state->listen_data); } void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - data = state->backend_data; + data = state->listen_data; data->shutdown_style = SHUTDOWN_EVENT; CFRunLoopStop(data->rl); @@ -460,9 +516,9 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) void fsm_listen__loop(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - data = state->backend_data; + data = state->listen_data; data->rl = CFRunLoopGetCurrent(); @@ -479,7 +535,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) switch (data->shutdown_style) { case FORCE_ERROR_STOP: - state->error_code = -1; + state->listen_error_code = -1; /* fall thru */ case FORCE_SHUTDOWN: ipc_server_stop_async(state->ipc_server_data); @@ -491,7 +547,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) return; force_error_stop_without_loop: - state->error_code = -1; + state->listen_error_code = -1; ipc_server_stop_async(state->ipc_server_data); return; } diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c index 9bb92935595151..eb619d3b980b66 100644 --- a/compat/fsmonitor/fsm-listen-win32.c +++ b/compat/fsmonitor/fsm-listen-win32.c @@ -25,6 +25,9 @@ struct one_watch DWORD count; struct strbuf path; + wchar_t wpath_longname[MAX_LONG_PATH + 1]; + DWORD wpath_longname_len; + HANDLE hDir; HANDLE hEvent; OVERLAPPED overlapped; @@ -34,9 +37,24 @@ struct one_watch * need to later call GetOverlappedResult() and possibly CancelIoEx(). */ BOOL is_active; + + /* + * Are shortnames enabled on the containing drive? This is + * always true for "C:/" drives and usually never true for + * other drives. + * + * We only set this for the worktree because we only need to + * convert shortname paths to longname paths for items we send + * to clients. (We don't care about shortname expansion for + * paths inside a GITDIR because we never send them to + * clients.) + */ + BOOL has_shortnames; + BOOL has_tilda; + wchar_t dotgit_shortname[16]; /* for 8.3 name */ }; -struct fsmonitor_daemon_backend_data +struct fsm_listen_data { struct one_watch *watch_worktree; struct one_watch *watch_gitdir; @@ -48,22 +66,21 @@ struct fsmonitor_daemon_backend_data #define LISTENER_HAVE_DATA_WORKTREE 1 #define LISTENER_HAVE_DATA_GITDIR 2 int nr_listener_handles; - - struct strbuf dot_git_shortname; }; /* - * Convert the WCHAR path from the notification into UTF8 and - * then normalize it. + * Convert the WCHAR path from the event into UTF8 and normalize it. + * + * `wpath_len` is in WCHARS not bytes. */ -static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, +static int normalize_path_in_utf8(wchar_t *wpath, DWORD wpath_len, struct strbuf *normalized_path) { int reserve; int len = 0; strbuf_reset(normalized_path); - if (!info->FileNameLength) + if (!wpath_len) goto normalize; /* @@ -72,12 +89,12 @@ static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, * sequence of 2 UTF8 characters. That should let us * avoid ERROR_INSUFFICIENT_BUFFER 99.9+% of the time. */ - reserve = info->FileNameLength + 1; + reserve = 2 * wpath_len + 1; strbuf_grow(normalized_path, reserve); for (;;) { - len = WideCharToMultiByte(CP_UTF8, 0, info->FileName, - info->FileNameLength / sizeof(WCHAR), + len = WideCharToMultiByte(CP_UTF8, 0, + wpath, wpath_len, normalized_path->buf, strbuf_avail(normalized_path) - 1, NULL, NULL); @@ -85,9 +102,7 @@ static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, goto normalize; if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) { error("[GLE %ld] could not convert path to UTF-8: '%.*ls'", - GetLastError(), - (int)(info->FileNameLength / sizeof(WCHAR)), - info->FileName); + GetLastError(), (int)wpath_len, wpath); return -1; } @@ -100,9 +115,151 @@ static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, return strbuf_normalize_path(normalized_path); } +/* + * See if the worktree root directory has shortnames enabled. + * This will help us decide if we need to do an expensive shortname + * to longname conversion on every notification event. + * + * We do not want to create a file to test this, so we assume that the + * root directory contains a ".git" file or directory. (Out caller + * only calls us for the worktree root, so this should be fine.) + * + * Remember the spelling of the shortname for ".git" if it exists. + */ +static void check_for_shortnames(struct one_watch *watch) +{ + wchar_t buf_in[MAX_LONG_PATH + 1]; + wchar_t buf_out[MAX_LONG_PATH + 1]; + wchar_t *last_slash = NULL; + wchar_t *last_bslash = NULL; + wchar_t *last; + + /* build L"/.git" */ + wcscpy(buf_in, watch->wpath_longname); + wcscpy(buf_in + watch->wpath_longname_len, L".git"); + + if (!GetShortPathNameW(buf_in, buf_out, MAX_LONG_PATH)) + return; + + last_slash = wcsrchr(buf_out, L'/'); + last_bslash = wcsrchr(buf_out, L'\\'); + if (last_slash > last_bslash) + last = last_slash + 1; + else if (last_bslash) + last = last_bslash + 1; + else + last = buf_out; + + if (!wcscmp(last, L".git")) + return; + + watch->has_shortnames = 1; + wcsncpy(watch->dotgit_shortname, last, + ARRAY_SIZE(watch->dotgit_shortname)); + + /* + * The shortname for ".git" is usually of the form "GIT~1", so + * we should be able to avoid shortname to longname mapping on + * every notification event if the source string does not + * contain a "~". + * + * However, the documentation for GetLongPathNameW() says + * that there are filesystems that don't follow that pattern + * and warns against this optimization. + * + * Lets test this. + */ + if (wcschr(watch->dotgit_shortname, L'~')) + watch->has_tilda = 1; +} + +enum get_relative_result { + GRR_NO_CONVERSION_NEEDED, + GRR_HAVE_CONVERSION, + GRR_SHUTDOWN, +}; + +/* + * Info notification paths are relative to the root of the watch. + * If our CWD is still at the root, then we can use relative paths + * to convert from shortnames to longnames. If our process has a + * different CWD, then we need to construct an absolute path, do + * the conversion, and then return the root-relative portion. + * + * We use the longname form of the root as our basis and assume that + * it already has a trailing slash. + * + * `wpath_len` is in WCHARS not bytes. + */ +static enum get_relative_result get_relative_longname( + struct one_watch *watch, + const wchar_t *wpath, DWORD wpath_len, + wchar_t *wpath_longname) +{ + wchar_t buf_in[2 * MAX_LONG_PATH + 1]; + wchar_t buf_out[MAX_LONG_PATH + 1]; + DWORD root_len; + + /* Build L"/" */ + root_len = watch->wpath_longname_len; + wcsncpy(buf_in, watch->wpath_longname, root_len); + wcsncpy(buf_in + root_len, wpath, wpath_len); + buf_in[root_len + wpath_len] = 0; + + /* + * We don't actually know if the source pathname is a + * shortname or a longname. This routine allows either to be + * given as input. + */ + if (!GetLongPathNameW(buf_in, buf_out, MAX_LONG_PATH)) { + /* + * The shortname to longname conversion can fail for + * various reasons, for example if the file has been + * deleted. (That is, if we just received a + * delete-file notification event and the file is + * already gone, we can't ask the file system to + * lookup the longname for it. Likewise, for moves + * and renames where we are given the old name.) + * + * NEEDSWORK: Since deleting or moving a file or + * directory by shortname is rather obscure, I'm going + * ignore the failure and ask the caller to report the + * original relative path. This seemds kinder than + * failing here and forcing a resync. + */ + return GRR_NO_CONVERSION_NEEDED; + } + + if (!wcscmp(buf_in, buf_out)) { + /* + * The path does not have a shortname alias. + */ + return GRR_NO_CONVERSION_NEEDED; + } + + if (wcsncmp(buf_in, buf_out, root_len)) { + /* + * The spelling of the root directory portion of the computed + * longname has changed. This should not happen. Basically, + * it means that we don't know where (without recomputing the + * longname of just the root directory) to split out the + * relative path. Since this should not happen, I'm just + * going to let this fail and force a shutdown (because all + * subsequent events are probably going to see the same + * mismatch). + */ + return GRR_SHUTDOWN; + } + + /* Return the worktree root-relative portion of the longname. */ + + wcscpy(wpath_longname, buf_out + root_len); + return GRR_HAVE_CONVERSION; +} + void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) { - SetEvent(state->backend_data->hListener[LISTENER_SHUTDOWN]); + SetEvent(state->listen_data->hListener[LISTENER_SHUTDOWN]); } static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, @@ -113,7 +270,9 @@ static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, DWORD share_mode = FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; HANDLE hDir; - wchar_t wpath[MAX_LONG_PATH]; + DWORD len_longname; + wchar_t wpath[MAX_LONG_PATH + 1]; + wchar_t wpath_longname[MAX_LONG_PATH + 1]; if (xutftowcs_long_path(wpath, path) < 0) { error(_("could not convert to wide characters: '%s'"), path); @@ -130,6 +289,20 @@ static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, return NULL; } + if (!GetLongPathNameW(wpath, wpath_longname, MAX_LONG_PATH)) { + error(_("[GLE %ld] could not get longname of '%s'"), + GetLastError(), path); + CloseHandle(hDir); + return NULL; + } + + len_longname = wcslen(wpath_longname); + if (wpath_longname[len_longname - 1] != L'/' && + wpath_longname[len_longname - 1] != L'\\') { + wpath_longname[len_longname++] = L'/'; + wpath_longname[len_longname] = 0; + } + CALLOC_ARRAY(watch, 1); watch->buf_len = sizeof(watch->buffer); /* assume full MAX_RDCW_BUF */ @@ -137,6 +310,9 @@ static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, strbuf_init(&watch->path, 0); strbuf_addstr(&watch->path, path); + wcscpy(watch->wpath_longname, wpath_longname); + watch->wpath_longname_len = len_longname; + watch->hDir = hDir; watch->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); @@ -157,7 +333,7 @@ static void destroy_watch(struct one_watch *watch) free(watch); } -static int start_rdcw_watch(struct fsmonitor_daemon_backend_data *data, +static int start_rdcw_watch(struct fsm_listen_data *data, struct one_watch *watch) { DWORD dwNotifyFilter = @@ -222,12 +398,22 @@ static int recv_rdcw_watch(struct one_watch *watch) } /* - * NEEDSWORK: If an external is deleted, the above - * returns an error. I'm not sure that there's anything that - * we can do here other than failing -- the /.git - * link file would be broken anyway. We might try to check - * for that and return a better error message, but I'm not - * sure it is worth it. + * GetOverlappedResult() fails if the watched directory is + * deleted while we were waiting for an overlapped IO to + * complete. The documentation did not list specific errors, + * but I observed ERROR_ACCESS_DENIED (0x05) errors during + * testing. + * + * Note that we only get notificaiton events for events + * *within* the directory, not *on* the directory itself. + * (These might be properies of the parent directory, for + * example). + * + * NEEDSWORK: We might try to check for the deleted directory + * case and return a better error message, but I'm not sure it + * is worth it. + * + * Shutdown if we get any error. */ error("GetOverlappedResult failed on '%s' [GLE %ld]", @@ -265,11 +451,11 @@ static void cancel_rdcw_watch(struct one_watch *watch) * Return 1 if we should shutdown. */ static int process_1_worktree_event( - FILE_NOTIFY_INFORMATION *info, struct string_list *cookie_list, struct fsmonitor_batch **batch, const struct strbuf *path, - enum fsmonitor_path_type t) + enum fsmonitor_path_type t, + DWORD info_action) { const char *slash; @@ -289,8 +475,8 @@ static int process_1_worktree_event( case IS_DOT_GIT: /* "/.git" was deleted (or renamed away) */ - if ((info->Action == FILE_ACTION_REMOVED) || - (info->Action == FILE_ACTION_RENAMED_OLD_NAME)) { + if ((info_action == FILE_ACTION_REMOVED) || + (info_action == FILE_ACTION_RENAMED_OLD_NAME)) { trace2_data_string("fsmonitor", NULL, "fsm-listen/dotgit", "removed"); @@ -326,12 +512,13 @@ static int process_1_worktree_event( */ static int process_worktree_events(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; struct one_watch *watch = data->watch_worktree; struct strbuf path = STRBUF_INIT; struct string_list cookie_list = STRING_LIST_INIT_DUP; struct fsmonitor_batch *batch = NULL; const char *p = watch->buffer; + wchar_t wpath_longname[MAX_LONG_PATH + 1]; /* * If the kernel gets more events than will fit in the kernel @@ -339,6 +526,10 @@ static int process_worktree_events(struct fsmonitor_daemon_state *state) * returns a count of zero. * * Yes, the call returns WITHOUT error and with length zero. + * This is the documented behavior. (My testing has confirmed + * that it also sets the last error to ERROR_NOTIFY_ENUM_DIR, + * but we do not rely on that since the function did not + * return an error and it is not documented.) * * (The "overflow" case is not ambiguous with the "no data" case * because we did an INFINITE wait.) @@ -360,52 +551,62 @@ static int process_worktree_events(struct fsmonitor_daemon_state *state) */ for (;;) { FILE_NOTIFY_INFORMATION *info = (void *)p; + wchar_t *wpath = info->FileName; + DWORD wpath_len = info->FileNameLength / sizeof(WCHAR); enum fsmonitor_path_type t; + enum get_relative_result grr; + + if (watch->has_shortnames) { + if (!wcscmp(wpath, watch->dotgit_shortname)) { + /* + * This event exactly matches the + * spelling of the shortname of + * ".git", so we can skip some steps. + * + * (This case is odd because the user + * can "rm -rf GIT~1" and we cannot + * use the filesystem to map it back + * to ".git".) + */ + strbuf_reset(&path); + strbuf_addstr(&path, ".git"); + t = IS_DOT_GIT; + goto process_it; + } + + if (watch->has_tilda && !wcschr(wpath, L'~')) { + /* + * Shortnames on this filesystem have tildas + * and the notification path does not have + * one, so we assume that it is a longname. + */ + goto normalize_it; + } + + grr = get_relative_longname(watch, wpath, wpath_len, + wpath_longname); + switch (grr) { + case GRR_NO_CONVERSION_NEEDED: /* use info buffer as is */ + break; + case GRR_HAVE_CONVERSION: + wpath = wpath_longname; + wpath_len = wcslen(wpath); + break; + default: + case GRR_SHUTDOWN: + goto force_shutdown; + } + } - strbuf_reset(&path); - if (normalize_path_in_utf8(info, &path) == -1) +normalize_it: + if (normalize_path_in_utf8(wpath, wpath_len, &path) == -1) goto skip_this_path; t = fsmonitor_classify_path_workdir_relative(path.buf); - if (process_1_worktree_event(info, &cookie_list, &batch, - &path, t)) - goto force_shutdown; - - /* - * NEEDSWORK: If `path` contains a shortname (that is, - * if any component within it is a shortname), we - * should expand it to a longname (See - * `GetLongPathNameW()`) and re-normalize, classify, - * and process it because our client is probably - * expecting "normal" paths. - * - * HOWEVER, if our process has called `chdir()` to get - * us out of the root of the worktree (so that the - * root directory is not busy), then we have to be - * careful to convert the paths in the INFO array - * (which are relative to the directory of the RDCW - * watch and not the CWD) into absolute paths before - * calling GetLongPathNameW() and then convert the - * computed value back to a RDCW-relative pathname - * (which is what we and the client expect). - * - * FOR NOW, just handle case (1) exactly so that we - * shutdown properly when ".git" is deleted via the - * shortname alias. - * - * We might see case (2) events for cookie files, but - * we can ignore them. - * - * FOR LATER, handle case (3) where the worktree - * events contain shortnames. We should convert - * them to longnames to avoid confusing the client. - */ - if (data->dot_git_shortname.len && - !strcmp(path.buf, data->dot_git_shortname.buf) && - process_1_worktree_event(info, &cookie_list, &batch, - &data->dot_git_shortname, - IS_DOT_GIT)) +process_it: + if (process_1_worktree_event(&cookie_list, &batch, &path, t, + info->Action)) goto force_shutdown; skip_this_path: @@ -421,7 +622,7 @@ static int process_worktree_events(struct fsmonitor_daemon_state *state) return LISTENER_HAVE_DATA_WORKTREE; force_shutdown: - fsmonitor_batch__pop(batch); + fsmonitor_batch__free_list(batch); string_list_clear(&cookie_list, 0); strbuf_release(&path); return LISTENER_SHUTDOWN; @@ -435,10 +636,13 @@ static int process_worktree_events(struct fsmonitor_daemon_state *state) * Note that we DO NOT get filesystem events on the external * itself (it is not inside something that we are watching). In particular, * we do not get an event if the external is deleted. + * + * Also, we do not care about shortnames within the external , since + * we never send these paths to clients. */ static int process_gitdir_events(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; struct one_watch *watch = data->watch_gitdir; struct strbuf path = STRBUF_INIT; struct string_list cookie_list = STRING_LIST_INIT_DUP; @@ -456,8 +660,10 @@ static int process_gitdir_events(struct fsmonitor_daemon_state *state) const char *slash; enum fsmonitor_path_type t; - strbuf_reset(&path); - if (normalize_path_in_utf8(info, &path) == -1) + if (normalize_path_in_utf8( + info->FileName, + info->FileNameLength / sizeof(WCHAR), + &path) == -1) goto skip_this_path; t = fsmonitor_classify_path_gitdir_relative(path.buf); @@ -480,14 +686,6 @@ static int process_gitdir_events(struct fsmonitor_daemon_state *state) t, path.buf); } - /* - * WRT shortnames, this external gitdir will not see - * case (1) nor case (3) events. - * - * We might see case (2) events for cookie files, but - * we can ignore them. - */ - skip_this_path: if (!info->NextEntryOffset) break; @@ -502,11 +700,11 @@ static int process_gitdir_events(struct fsmonitor_daemon_state *state) void fsm_listen__loop(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; DWORD dwWait; int result; - state->error_code = 0; + state->listen_error_code = 0; if (start_rdcw_watch(data, data->watch_worktree) == -1) goto force_error_stop; @@ -571,7 +769,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) } force_error_stop: - state->error_code = -1; + state->listen_error_code = -1; force_shutdown: /* @@ -588,8 +786,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) int fsm_listen__ctor(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; - char shortname[16]; /* a padded 8.3 buffer */ + struct fsm_listen_data *data; CALLOC_ARRAY(data, 1); @@ -600,6 +797,8 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) if (!data->watch_worktree) goto failed; + check_for_shortnames(data->watch_worktree); + if (state->nr_paths_watching > 1) { data->watch_gitdir = create_watch(state, state->path_gitdir_watch.buf); @@ -620,53 +819,7 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) data->nr_listener_handles++; } - /* - * NEEDSWORK: Properly handle 8.3 shortnames. RDCW events can - * contain a shortname (if another application uses a - * shortname in a system call). We care about aliasing and - * the use of shortnames for: - * - * (1) ".git", - * -- if an external process deletes ".git" using "GIT~1", - * we need to catch that and shutdown. - * - * (2) our cookie files, - * -- if an external process deletes one of our cookie - * files using a shortname, we will get a shortname - * event for it. However, we should have already - * gotten a longname event for it when we created the - * cookie, so we can safely discard the shortname - * events for cookie files. - * - * (3) the spelling of modified files that we report to clients. - * -- we need to report the longname to the client because - * that is what they are expecting. Presumably, the - * client is going to lookup the paths that we report - * in their index and untracked-cache, so we should - * normalize the data for them. (Technically, they - * could adapt, so we could relax this maybe.) - * - * FOR NOW, while our CWD is at the root of the worktree we - * can easily get the spelling of the shortname of ".git" (if - * the volume has shortnames enabled). For most worktrees - * this value will be "GIT~1", but we don't want to assume - * that. - * - * Capture this so that we can handle (1). - * - * We leave (3) for a future effort. - */ - strbuf_init(&data->dot_git_shortname, 0); - GetShortPathNameA(".git", shortname, sizeof(shortname)); - if (!strcmp(".git", shortname)) - trace_printf_key(&trace_fsmonitor, "No shortname for '.git'"); - else { - trace_printf_key(&trace_fsmonitor, - "Shortname of '.git' is '%s'", shortname); - strbuf_addstr(&data->dot_git_shortname, shortname); - } - - state->backend_data = data; + state->listen_data = data; return 0; failed: @@ -679,16 +832,16 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) void fsm_listen__dtor(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - if (!state || !state->backend_data) + if (!state || !state->listen_data) return; - data = state->backend_data; + data = state->listen_data; CloseHandle(data->hEventShutdown); destroy_watch(data->watch_worktree); destroy_watch(data->watch_gitdir); - FREE_AND_NULL(state->backend_data); + FREE_AND_NULL(state->listen_data); } diff --git a/compat/fsmonitor/fsm-listen.h b/compat/fsmonitor/fsm-listen.h index f0539349baf848..41650bf8972217 100644 --- a/compat/fsmonitor/fsm-listen.h +++ b/compat/fsmonitor/fsm-listen.h @@ -33,7 +33,7 @@ void fsm_listen__dtor(struct fsmonitor_daemon_state *state); * do so if the listener thread receives a normal shutdown signal from * the IPC layer.) * - * It should set `state->error_code` to -1 if the daemon should exit + * It should set `state->listen_error_code` to -1 if the daemon should exit * with an error. */ void fsm_listen__loop(struct fsmonitor_daemon_state *state); diff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c index 83d91f85ece3fa..d775c082451d8a 100644 --- a/compat/fsmonitor/fsm-settings-win32.c +++ b/compat/fsmonitor/fsm-settings-win32.c @@ -108,7 +108,7 @@ static enum fsmonitor_reason is_remote(struct repository *r) driveType = GetDriveTypeW(wfullpath); trace_printf_key(&trace_fsmonitor, - "DriveType '%s' L'%S' (%u)", + "DriveType '%s' L'%ls' (%u)", r->worktree, wfullpath, driveType); if (driveType == DRIVE_REMOTE) { diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c index 6c8a308de1351b..20ea7b65e0ba63 100644 --- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c @@ -3,6 +3,8 @@ #include "strbuf.h" #include "pkt-line.h" #include "thread-utils.h" +#include "accctrl.h" +#include "aclapi.h" #ifndef SUPPORTS_SIMPLE_IPC /* @@ -112,13 +114,14 @@ static enum ipc_active_state connect_to_server( t_start_ms = (DWORD)(getnanotime() / 1000000); if (!WaitNamedPipeW(wpath, timeout_ms)) { - if (GetLastError() == ERROR_SEM_TIMEOUT) + DWORD gleWait = GetLastError(); + + if (gleWait == ERROR_SEM_TIMEOUT) return IPC_STATE__NOT_LISTENING; - gle = GetLastError(); trace2_data_intmax("ipc-debug", NULL, "connect/waitpipe/gle", - (intmax_t)gle); + (intmax_t)gleWait); return IPC_STATE__OTHER_ERROR; } @@ -591,11 +594,132 @@ static void *server_thread_proc(void *_server_thread_data) return NULL; } +/* + * We need to build a Windows "SECURITY_ATTRIBUTES" object and use it + * to apply an ACL when we create the initial instance of the Named + * Pipe. The construction is somewhat involved and consists of + * several sequential steps and intermediate objects. + * + * We use this structure to hold these intermediate pointers so that + * we can free them as a group. (It is unclear from the docs whether + * some of these intermediate pointers can be freed before we are + * finished using the "lpSA" member.) + */ +struct my_sa_data +{ + PSID pEveryoneSID; + PACL pACL; + PSECURITY_DESCRIPTOR pSD; + LPSECURITY_ATTRIBUTES lpSA; +}; + +static void init_sa(struct my_sa_data *d) +{ + memset(d, 0, sizeof(*d)); +} + +static void release_sa(struct my_sa_data *d) +{ + if (d->pEveryoneSID) + FreeSid(d->pEveryoneSID); + if (d->pACL) + LocalFree(d->pACL); + if (d->pSD) + LocalFree(d->pSD); + if (d->lpSA) + LocalFree(d->lpSA); + + memset(d, 0, sizeof(*d)); +} + +/* + * Create SECURITY_ATTRIBUTES to apply to the initial named pipe. The + * creator of the first server instance gets to set the ACLs on it. + * + * We allow the well-known group `EVERYONE` to have read+write access + * to the named pipe so that clients can send queries to the daemon + * and receive the response. + * + * Normally, this is not necessary since the daemon is usually + * automatically started by a foreground command like `git status`, + * but in those cases where an elevated Git command started the daemon + * (such that the daemon itself runs with elevation), we need to add + * the ACL so that non-elevated commands can write to it. + * + * The following document was helpful: + * https://docs.microsoft.com/en-us/windows/win32/secauthz/creating-a-security-descriptor-for-a-new-object-in-c-- + * + * Returns d->lpSA set to a SA or NULL. + */ +static LPSECURITY_ATTRIBUTES get_sa(struct my_sa_data *d) +{ + SID_IDENTIFIER_AUTHORITY sid_auth_world = SECURITY_WORLD_SID_AUTHORITY; +#define NR_EA (1) + EXPLICIT_ACCESS ea[NR_EA]; + DWORD dwResult; + + if (!AllocateAndInitializeSid(&sid_auth_world, 1, + SECURITY_WORLD_RID, 0,0,0,0,0,0,0, + &d->pEveryoneSID)) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "alloc-world-sid/gle", + (intmax_t)gle); + goto fail; + } + + memset(ea, 0, NR_EA * sizeof(EXPLICIT_ACCESS)); + + ea[0].grfAccessPermissions = GENERIC_READ | GENERIC_WRITE; + ea[0].grfAccessMode = SET_ACCESS; + ea[0].grfInheritance = NO_INHERITANCE; + ea[0].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE; + ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID; + ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP; + ea[0].Trustee.ptstrName = (LPTSTR)d->pEveryoneSID; + + dwResult = SetEntriesInAcl(NR_EA, ea, NULL, &d->pACL); + if (dwResult != ERROR_SUCCESS) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "set-acl-entry/gle", + (intmax_t)gle); + trace2_data_intmax("ipc-debug", NULL, "set-acl-entry/dw", + (intmax_t)dwResult); + goto fail; + } + + d->pSD = (PSECURITY_DESCRIPTOR)LocalAlloc( + LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH); + if (!InitializeSecurityDescriptor(d->pSD, SECURITY_DESCRIPTOR_REVISION)) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "init-sd/gle", (intmax_t)gle); + goto fail; + } + + if (!SetSecurityDescriptorDacl(d->pSD, TRUE, d->pACL, FALSE)) { + DWORD gle = GetLastError(); + trace2_data_intmax("ipc-debug", NULL, "set-sd-dacl/gle", (intmax_t)gle); + goto fail; + } + + d->lpSA = (LPSECURITY_ATTRIBUTES)LocalAlloc(LPTR, sizeof(SECURITY_ATTRIBUTES)); + d->lpSA->nLength = sizeof(SECURITY_ATTRIBUTES); + d->lpSA->lpSecurityDescriptor = d->pSD; + d->lpSA->bInheritHandle = FALSE; + + return d->lpSA; + +fail: + release_sa(d); + return NULL; +} + static HANDLE create_new_pipe(wchar_t *wpath, int is_first) { HANDLE hPipe; DWORD dwOpenMode, dwPipeMode; - LPSECURITY_ATTRIBUTES lpsa = NULL; + struct my_sa_data my_sa_data; + + init_sa(&my_sa_data); dwOpenMode = PIPE_ACCESS_INBOUND | PIPE_ACCESS_OUTBOUND | FILE_FLAG_OVERLAPPED; @@ -611,20 +735,15 @@ static HANDLE create_new_pipe(wchar_t *wpath, int is_first) * set the ACL / Security Attributes on the named * pipe; subsequent instances inherit and cannot * change them. - * - * TODO Should we allow the application layer to - * specify security attributes, such as `LocalService` - * or `LocalSystem`, when we create the named pipe? - * This question is probably not important when the - * daemon is started by a foreground user process and - * only needs to talk to the current user, but may be - * if the daemon is run via the Control Panel as a - * System Service. */ + get_sa(&my_sa_data); } hPipe = CreateNamedPipeW(wpath, dwOpenMode, dwPipeMode, - PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, lpsa); + PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, + my_sa_data.lpSA); + + release_sa(&my_sa_data); return hPipe; } diff --git a/config.mak.uname b/config.mak.uname index 3aa2e00d7067a8..5deac1b450c2a4 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -147,8 +147,16 @@ ifeq ($(uname_S),Darwin) MSGFMT = /usr/local/opt/gettext/bin/msgfmt endif endif + + # The builtin FSMonitor on MacOS builds upon Simple-IPC. Both require + # Unix domain sockets and PThreads. + ifndef NO_PTHREADS + ifndef NO_UNIX_SOCKETS FSMONITOR_DAEMON_BACKEND = darwin FSMONITOR_OS_SETTINGS = darwin + endif + endif + BASIC_LDFLAGS += -framework CoreServices endif ifeq ($(uname_S),SunOS) @@ -423,8 +431,13 @@ ifeq ($(uname_S),Windows) # so we don't need this: # # SNPRINTF_RETURNS_BOGUS = YesPlease + + # The builtin FSMonitor requires Named Pipes and Threads on Windows. + # These are always available, so we do not have to conditionally + # support it. FSMONITOR_DAEMON_BACKEND = win32 FSMONITOR_OS_SETTINGS = win32 + NO_SVN_TESTS = YesPlease RUNTIME_PREFIX = YesPlease HAVE_WPGMPTR = YesWeDo @@ -612,8 +625,13 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_STRTOUMAX = YesPlease NO_MKDTEMP = YesPlease NO_SVN_TESTS = YesPlease + + # The builtin FSMonitor requires Named Pipes and Threads on Windows. + # These are always available, so we do not have to conditionally + # support it. FSMONITOR_DAEMON_BACKEND = win32 FSMONITOR_OS_SETTINGS = win32 + RUNTIME_PREFIX = YesPlease HAVE_WPGMPTR = YesWeDo NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 75f7bde880c74f..66667cf608ce3f 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -292,20 +292,22 @@ else() endif() endif() -if(CMAKE_SYSTEM_NAME STREQUAL "Windows") - add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) -elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") - add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) -endif() - -if(CMAKE_SYSTEM_NAME STREQUAL "Windows") - add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c) -elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") - add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c) +if(SUPPORTS_SIMPLE_IPC) + if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + + add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c) + elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") + add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + + add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c) + endif() endif() set(EXE_EXTENSION ${CMAKE_EXECUTABLE_SUFFIX}) diff --git a/fsmonitor--daemon.h b/fsmonitor--daemon.h index e9fc099bae9cfb..716e0e4d28dfa5 100644 --- a/fsmonitor--daemon.h +++ b/fsmonitor--daemon.h @@ -20,9 +20,9 @@ struct fsmonitor_token_data; struct fsmonitor_batch *fsmonitor_batch__new(void); /* - * Free this batch and return the value of the batch->next field. + * Free the list of batches starting with this one. */ -struct fsmonitor_batch *fsmonitor_batch__pop(struct fsmonitor_batch *batch); +void fsmonitor_batch__free_list(struct fsmonitor_batch *batch); /* * Add this path to this batch of modified files. @@ -33,10 +33,12 @@ struct fsmonitor_batch *fsmonitor_batch__pop(struct fsmonitor_batch *batch); */ void fsmonitor_batch__add_path(struct fsmonitor_batch *batch, const char *path); -struct fsmonitor_daemon_backend_data; /* opaque platform-specific data */ +struct fsm_listen_data; /* opaque platform-specific data for listener thread */ +struct fsm_health_data; /* opaque platform-specific data for health thread */ struct fsmonitor_daemon_state { pthread_t listener_thread; + pthread_t health_thread; pthread_mutex_t main_lock; struct strbuf path_worktree_watch; @@ -50,10 +52,13 @@ struct fsmonitor_daemon_state { int cookie_seq; struct hashmap cookies; - int error_code; - struct fsmonitor_daemon_backend_data *backend_data; + int listen_error_code; + int health_error_code; + struct fsm_listen_data *listen_data; + struct fsm_health_data *health_data; struct ipc_server_data *ipc_server_data; + struct strbuf path_ipc; }; /* diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c index 6cbfa1b282fbfc..52a3392e16e465 100644 --- a/fsmonitor-ipc.c +++ b/fsmonitor-ipc.c @@ -69,9 +69,6 @@ int fsmonitor_ipc__send_query(const char *since_token, goto done; case IPC_STATE__NOT_LISTENING: - ret = error(_("fsmonitor_ipc__send_query: daemon not available")); - goto done; - case IPC_STATE__PATH_NOT_FOUND: if (tried_to_spawn) goto done; diff --git a/fsmonitor-ipc.h b/fsmonitor-ipc.h index 837c5e5b64ad8c..b6a7067c3af511 100644 --- a/fsmonitor-ipc.h +++ b/fsmonitor-ipc.h @@ -1,6 +1,8 @@ #ifndef FSMONITOR_IPC_H #define FSMONITOR_IPC_H +#include "simple-ipc.h" + /* * Returns true if built-in file system monitor daemon is defined * for this platform. @@ -26,8 +28,6 @@ enum ipc_active_state fsmonitor_ipc__get_state(void); * Connect to a `git-fsmonitor--daemon` process via simple-ipc * and ask for the set of changed files since the given token. * - * This DOES NOT use the hook interface. - * * Spawn a daemon process in the background if necessary. * * Returns -1 on error; 0 on success. diff --git a/fsmonitor.c b/fsmonitor.c index 3a63cda58f73f0..b973e30aa98485 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -199,25 +199,24 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name) { int i, len = strlen(name); if (name[len - 1] == '/') { - - /* - * TODO We should binary search to find the first path with - * TODO this directory prefix. Then linearly update entries - * TODO while the prefix matches. Taking care to search without - * TODO the trailing slash -- because '/' sorts after a few - * TODO interesting special chars, like '.' and ' '. - */ + const char *rel; + int pos = index_name_pos(istate, name, len); + if (pos < 0) + pos = -pos - 1; /* Mark all entries for the folder invalid */ - for (i = 0; i < istate->cache_nr; i++) { - if (istate->cache[i]->ce_flags & CE_FSMONITOR_VALID && - starts_with(istate->cache[i]->name, name)) + for (i = pos; i < istate->cache_nr; i++) { + if (!starts_with(istate->cache[i]->name, name)) + break; + /* Only mark the immediate children in the folder */ + rel = istate->cache[i]->name + len; + if (!strchr(rel, '/')) istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID; } /* Need to remove the / from the path for the untracked cache */ name[len - 1] = '\0'; } else { - int pos = index_name_pos(istate, name, strlen(name)); + int pos = index_name_pos(istate, name, len); if (pos >= 0) { struct cache_entry *ce = istate->cache[pos]; @@ -403,6 +402,8 @@ void refresh_fsmonitor(struct index_state *istate) * information and that we should consider everything * invalid. We call this a trivial response. */ + trace2_region_enter("fsmonitor", "apply_results", istate->repo); + if (query_success && query_result.buf[bol] != '/') { /* * Mark all pathnames returned by the monitor as dirty. @@ -431,6 +432,9 @@ void refresh_fsmonitor(struct index_state *istate) if (count > fsmonitor_force_update_threshold) istate->cache_changed |= FSMONITOR_CHANGED; + trace2_data_intmax("fsmonitor", istate->repo, "apply_count", + count); + } else { /* * We received a trivial response, so invalidate everything. @@ -458,6 +462,8 @@ void refresh_fsmonitor(struct index_state *istate) if (istate->untracked) istate->untracked->use_fsmonitor = 0; } + trace2_region_leave("fsmonitor", "apply_results", istate->repo); + strbuf_release(&query_result); /* Now that we've updated istate, save the last_update_token */ diff --git a/repo-settings.c b/repo-settings.c index 21317070a7a69d..9a6c974c96a92e 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -74,7 +74,28 @@ void prepare_repo_settings(struct repository *r) if (!repo_config_get_bool(r, "feature.experimental", &value) && value) { UPDATE_DEFAULT_BOOL(r->settings.fetch_negotiation_algorithm, FETCH_NEGOTIATION_SKIPPING); - if (feature_many_files && fsmonitor_ipc__is_supported()) + + /* + * Force enable the builtin FSMonitor (unless the repo + * is incompatible or they've already selected it or + * the hook version). But only if they haven't + * explicitly turned it off -- so only if our config + * value is UNSET. + * + * lookup_fsmonitor_settings() and check_for_ipc() do + * not distinguish between explicitly set FALSE and + * UNSET, so we re-test for an UNSET config key here. + * + * I'm not sure I want to fix fsmonitor-settings.c to + * have more than one _DISABLED state since our usage + * here is only to support this experimental period + * (and I don't want to overload the _reason field + * because it describes incompabilities). + */ + if (feature_many_files && + fsmonitor_ipc__is_supported() && + fsm_settings__get_mode(r) == FSMONITOR_MODE_DISABLED && + repo_config_get_bool(r, "core.usebuiltinfsmonitor", &value)) fsm_settings__set_ipc(r); } diff --git a/run-command.c b/run-command.c index 9f48636ea07541..37b7adb13ee465 100644 --- a/run-command.c +++ b/run-command.c @@ -1985,3 +1985,132 @@ void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir) } strvec_pushf(env_array, "%s=%s", GIT_DIR_ENVIRONMENT, new_git_dir); } + +enum start_bg_result start_bg_command(struct child_process *cmd, + start_bg_wait_cb *wait_cb, + void *cb_data, + unsigned int timeout_sec) +{ + enum start_bg_result sbgr = SBGR_ERROR; + int ret; + int wait_status; + pid_t pid_seen; + time_t time_limit; + + /* + * We do not allow clean-on-exit because the child process + * should persist in the background and possibly/probably + * after this process exits. So we don't want to kill the + * child during our atexit routine. + */ + if (cmd->clean_on_exit) + BUG("start_bg_command() does not allow non-zero clean_on_exit"); + + if (!cmd->trace2_child_class) + cmd->trace2_child_class = "background"; + + ret = start_command(cmd); + if (ret) { + /* + * We assume that if `start_command()` fails, we + * either get a complete `trace2_child_start() / + * trace2_child_exit()` pair or it fails before the + * `trace2_child_start()` is emitted, so we do not + * need to worry about it here. + * + * We also assume that `start_command()` does not add + * us to the cleanup list. And that it calls + * calls `child_process_clear()`. + */ + sbgr = SBGR_ERROR; + goto done; + } + + time(&time_limit); + time_limit += timeout_sec; + +wait: + pid_seen = waitpid(cmd->pid, &wait_status, WNOHANG); + + if (!pid_seen) { + /* + * The child is currently running. Ask the callback + * if the child is ready to do work or whether we + * should keep waiting for it to boot up. + */ + ret = (*wait_cb)(cmd, cb_data); + if (!ret) { + /* + * The child is running and "ready". + */ + trace2_child_ready(cmd, "ready"); + sbgr = SBGR_READY; + goto done; + } else if (ret > 0) { + /* + * The callback said to give it more time to boot up + * (subject to our timeout limit). + */ + time_t now; + + time(&now); + if (now < time_limit) + goto wait; + + /* + * Our timeout has expired. We don't try to + * kill the child, but rather let it continue + * (hopefully) trying to startup. + */ + trace2_child_ready(cmd, "timeout"); + sbgr = SBGR_TIMEOUT; + goto done; + } else { + /* + * The cb gave up on this child. It is still running, + * but our cb got an error trying to probe it. + */ + trace2_child_ready(cmd, "error"); + sbgr = SBGR_CB_ERROR; + goto done; + } + } + + else if (pid_seen == cmd->pid) { + int child_code = -1; + + /* + * The child started, but exited or was terminated + * before becoming "ready". + * + * We try to match the behavior of `wait_or_whine()` + * WRT the handling of WIFSIGNALED() and WIFEXITED() + * and convert the child's status to a return code for + * tracing purposes and emit the `trace2_child_exit()` + * event. + * + * We do not want the wait_or_whine() error message + * because we will be called by client-side library + * routines. + */ + if (WIFEXITED(wait_status)) + child_code = WEXITSTATUS(wait_status); + else if (WIFSIGNALED(wait_status)) + child_code = WTERMSIG(wait_status) + 128; + trace2_child_exit(cmd, child_code); + + sbgr = SBGR_DIED; + goto done; + } + + else if (pid_seen < 0 && errno == EINTR) + goto wait; + + trace2_child_exit(cmd, -1); + sbgr = SBGR_ERROR; + +done: + child_process_clear(cmd); + invalidate_lstat_cache(); + return sbgr; +} diff --git a/run-command.h b/run-command.h index 3fedd861246dbd..734a3fd8c530fa 100644 --- a/run-command.h +++ b/run-command.h @@ -498,4 +498,61 @@ int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn, */ void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir); +/** + * Possible return values for start_bg_command(). + */ +enum start_bg_result { + /* child process is "ready" */ + SBGR_READY = 0, + + /* child process could not be started */ + SBGR_ERROR, + + /* callback error when testing for "ready" */ + SBGR_CB_ERROR, + + /* timeout expired waiting for child to become "ready" */ + SBGR_TIMEOUT, + + /* child process exited or was signalled before becomming "ready" */ + SBGR_DIED, +}; + +/** + * Callback used by start_bg_command() to ask whether the + * child process is ready or needs more time to become "ready". + * + * The callback will receive the cmd and cb_data arguments given to + * start_bg_command(). + * + * Returns 1 is child needs more time (subject to the requested timeout). + * Returns 0 if child is "ready". + * Returns -1 on any error and cause start_bg_command() to also error out. + */ +typedef int(start_bg_wait_cb)(const struct child_process *cmd, void *cb_data); + +/** + * Start a command in the background. Wait long enough for the child + * to become "ready" (as defined by the provided callback). Capture + * immediate errors (like failure to start) and any immediate exit + * status (such as a shutdown/signal before the child became "ready") + * and return this like start_command(). + * + * We run a custom wait loop using the provided callback to wait for + * the child to start and become "ready". This is limited by the given + * timeout value. + * + * If the child does successfully start and become "ready", we orphan + * it into the background. + * + * The caller must not call finish_command(). + * + * The opaque cb_data argument will be forwarded to the callback for + * any instance data that it might require. This may be NULL. + */ +enum start_bg_result start_bg_command(struct child_process *cmd, + start_bg_wait_cb *wait_cb, + void *cb_data, + unsigned int timeout_sec); + #endif diff --git a/simple-ipc.h b/simple-ipc.h index 9c7330fcda0056..b396293bdfc553 100644 --- a/simple-ipc.h +++ b/simple-ipc.h @@ -5,13 +5,6 @@ * See Documentation/technical/api-simple-ipc.txt */ -#ifdef SUPPORTS_SIMPLE_IPC -#include "pkt-line.h" - -/* - * Simple IPC Client Side API. - */ - enum ipc_active_state { /* * The pipe/socket exists and the daemon is waiting for connections. @@ -43,6 +36,13 @@ enum ipc_active_state { IPC_STATE__OTHER_ERROR, }; +#ifdef SUPPORTS_SIMPLE_IPC +#include "pkt-line.h" + +/* + * Simple IPC Client Side API. + */ + struct ipc_client_connect_options { /* * Spin under timeout if the server is running but can't diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c index 9134518075097e..28365ff85b69bb 100644 --- a/t/helper/test-simple-ipc.c +++ b/t/helper/test-simple-ipc.c @@ -9,6 +9,7 @@ #include "parse-options.h" #include "thread-utils.h" #include "strvec.h" +#include "run-command.h" #ifndef SUPPORTS_SIMPLE_IPC int cmd__simple_ipc(int argc, const char **argv) @@ -267,185 +268,71 @@ static int daemon__run_server(void) */ ret = ipc_server_run(cl_args.path, &opts, test_app_cb, (void*)&my_app_data); if (ret == -2) - error(_("socket/pipe already in use: '%s'"), cl_args.path); + error("socket/pipe already in use: '%s'", cl_args.path); else if (ret == -1) - error_errno(_("could not start server on: '%s'"), cl_args.path); + error_errno("could not start server on: '%s'", cl_args.path); return ret; } -#ifndef GIT_WINDOWS_NATIVE -/* - * This is adapted from `daemonize()`. Use `fork()` to directly create and - * run the daemon in a child process. - */ -static int spawn_server(pid_t *pid) -{ - struct ipc_server_opts opts = { - .nr_threads = cl_args.nr_threads, - }; +static start_bg_wait_cb bg_wait_cb; - *pid = fork(); - - switch (*pid) { - case 0: - if (setsid() == -1) - error_errno(_("setsid failed")); - close(0); - close(1); - close(2); - sanitize_stdfds(); +static int bg_wait_cb(const struct child_process *cp, void *cb_data) +{ + int s = ipc_get_active_state(cl_args.path); - return ipc_server_run(cl_args.path, &opts, test_app_cb, - (void*)&my_app_data); + switch (s) { + case IPC_STATE__LISTENING: + /* child is "ready" */ + return 0; - case -1: - return error_errno(_("could not spawn daemon in the background")); + case IPC_STATE__NOT_LISTENING: + case IPC_STATE__PATH_NOT_FOUND: + /* give child more time */ + return 1; default: - return 0; + case IPC_STATE__INVALID_PATH: + case IPC_STATE__OTHER_ERROR: + /* all the time in world won't help */ + return -1; } } -#else -/* - * Conceptually like `daemonize()` but different because Windows does not - * have `fork(2)`. Spawn a normal Windows child process but without the - * limitations of `start_command()` and `finish_command()`. - */ -static int spawn_server(pid_t *pid) -{ - char test_tool_exe[MAX_PATH]; - struct strvec args = STRVEC_INIT; - int in, out; - - GetModuleFileNameA(NULL, test_tool_exe, MAX_PATH); - - in = open("/dev/null", O_RDONLY); - out = open("/dev/null", O_WRONLY); - - strvec_push(&args, test_tool_exe); - strvec_push(&args, "simple-ipc"); - strvec_push(&args, "run-daemon"); - strvec_pushf(&args, "--name=%s", cl_args.path); - strvec_pushf(&args, "--threads=%d", cl_args.nr_threads); - - *pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out); - close(in); - close(out); - - strvec_clear(&args); - if (*pid < 0) - return error(_("could not spawn daemon in the background")); - - return 0; -} -#endif - -/* - * This is adapted from `wait_or_whine()`. Watch the child process and - * let it get started and begin listening for requests on the socket - * before reporting our success. - */ -static int wait_for_server_startup(pid_t pid_child) +static int daemon__start_server(void) { - int status; - pid_t pid_seen; - enum ipc_active_state s; - time_t time_limit, now; + struct child_process cp = CHILD_PROCESS_INIT; + enum start_bg_result sbgr; - time(&time_limit); - time_limit += cl_args.max_wait_sec; + strvec_push(&cp.args, "test-tool"); + strvec_push(&cp.args, "simple-ipc"); + strvec_push(&cp.args, "run-daemon"); + strvec_pushf(&cp.args, "--name=%s", cl_args.path); + strvec_pushf(&cp.args, "--threads=%d", cl_args.nr_threads); - for (;;) { - pid_seen = waitpid(pid_child, &status, WNOHANG); + cp.no_stdin = 1; + cp.no_stdout = 1; + cp.no_stderr = 1; - if (pid_seen == -1) - return error_errno(_("waitpid failed")); + sbgr = start_bg_command(&cp, bg_wait_cb, NULL, cl_args.max_wait_sec); - else if (pid_seen == 0) { - /* - * The child is still running (this should be - * the normal case). Try to connect to it on - * the socket and see if it is ready for - * business. - * - * If there is another daemon already running, - * our child will fail to start (possibly - * after a timeout on the lock), but we don't - * care (who responds) if the socket is live. - */ - s = ipc_get_active_state(cl_args.path); - if (s == IPC_STATE__LISTENING) - return 0; - - time(&now); - if (now > time_limit) - return error(_("daemon not online yet")); - - continue; - } + switch (sbgr) { + case SBGR_READY: + return 0; - else if (pid_seen == pid_child) { - /* - * The new child daemon process shutdown while - * it was starting up, so it is not listening - * on the socket. - * - * Try to ping the socket in the odd chance - * that another daemon started (or was already - * running) while our child was starting. - * - * Again, we don't care who services the socket. - */ - s = ipc_get_active_state(cl_args.path); - if (s == IPC_STATE__LISTENING) - return 0; + default: + case SBGR_ERROR: + case SBGR_CB_ERROR: + return error("daemon failed to start"); - /* - * We don't care about the WEXITSTATUS() nor - * any of the WIF*(status) values because - * `cmd__simple_ipc()` does the `!!result` - * trick on all function return values. - * - * So it is sufficient to just report the - * early shutdown as an error. - */ - return error(_("daemon failed to start")); - } + case SBGR_TIMEOUT: + return error("daemon not online yet"); - else - return error(_("waitpid is confused")); + case SBGR_DIED: + return error("daemon terminated"); } } -/* - * This process will start a simple-ipc server in a background process and - * wait for it to become ready. This is like `daemonize()` but gives us - * more control and better error reporting (and makes it easier to write - * unit tests). - */ -static int daemon__start_server(void) -{ - pid_t pid_child; - int ret; - - /* - * Run the actual daemon in a background process. - */ - ret = spawn_server(&pid_child); - if (pid_child <= 0) - return ret; - - /* - * Let the parent wait for the child process to get started - * and begin listening for requests on the socket. - */ - ret = wait_for_server_startup(pid_child); - - return ret; -} - /* * This process will run a quick probe to see if a simple-ipc server * is active on this path. @@ -548,7 +435,7 @@ static int client__stop_server(void) time(&now); if (now > time_limit) - return error(_("daemon has not shutdown yet")); + return error("daemon has not shutdown yet"); } } diff --git a/trace2.c b/trace2.c index 256120c7fd553e..5e3a32049a830b 100644 --- a/trace2.c +++ b/trace2.c @@ -381,6 +381,37 @@ void trace2_child_exit_fl(const char *file, int line, struct child_process *cmd, us_elapsed_child); } +void trace2_child_ready_fl(const char *file, int line, + struct child_process *cmd, + const char *ready) +{ + struct tr2_tgt *tgt_j; + int j; + uint64_t us_now; + uint64_t us_elapsed_absolute; + uint64_t us_elapsed_child; + + if (!trace2_enabled) + return; + + us_now = getnanotime() / 1000; + us_elapsed_absolute = tr2tls_absolute_elapsed(us_now); + + if (cmd->trace2_child_us_start) + us_elapsed_child = us_now - cmd->trace2_child_us_start; + else + us_elapsed_child = 0; + + for_each_wanted_builtin (j, tgt_j) + if (tgt_j->pfn_child_ready_fl) + tgt_j->pfn_child_ready_fl(file, line, + us_elapsed_absolute, + cmd->trace2_child_id, + cmd->pid, + ready, + us_elapsed_child); +} + int trace2_exec_fl(const char *file, int line, const char *exe, const char **argv) { diff --git a/trace2.h b/trace2.h index ede18c2e06373f..c7250a43b9b5d2 100644 --- a/trace2.h +++ b/trace2.h @@ -243,6 +243,31 @@ void trace2_child_exit_fl(const char *file, int line, struct child_process *cmd, #define trace2_child_exit(cmd, code) \ trace2_child_exit_fl(__FILE__, __LINE__, (cmd), (code)) +/** + * Emits a "child_ready" message containing the "child-id" and a flag + * indicating whether the child was considered "ready" when we + * released it. + * + * This function should be called after starting a daemon process in + * the background (and after giving it sufficient time to boot + * up) to indicate that we no longer control or own it. + * + * The "ready" argument should contain one of { "ready", "timeout", + * "error" } to indicate the state of the running daemon when we + * released it. + * + * If the daemon process fails to start or it exits or is terminated + * while we are still waiting for it, the caller should emit a + * regular "child_exit" to report the normal process exit information. + * + */ +void trace2_child_ready_fl(const char *file, int line, + struct child_process *cmd, + const char *ready); + +#define trace2_child_ready(cmd, ready) \ + trace2_child_ready_fl(__FILE__, __LINE__, (cmd), (ready)) + /** * Emit an 'exec' event prior to calling one of exec(), execv(), * execvp(), and etc. On Unix-derived systems, this will be the diff --git a/trace2/tr2_tgt.h b/trace2/tr2_tgt.h index 7b904692123e28..83dba733535133 100644 --- a/trace2/tr2_tgt.h +++ b/trace2/tr2_tgt.h @@ -43,6 +43,10 @@ typedef void(tr2_tgt_evt_child_exit_fl_t)(const char *file, int line, uint64_t us_elapsed_absolute, int cid, int pid, int code, uint64_t us_elapsed_child); +typedef void(tr2_tgt_evt_child_ready_fl_t)(const char *file, int line, + uint64_t us_elapsed_absolute, + int cid, int pid, const char *ready, + uint64_t us_elapsed_child); typedef void(tr2_tgt_evt_thread_start_fl_t)(const char *file, int line, uint64_t us_elapsed_absolute); @@ -113,6 +117,7 @@ struct tr2_tgt { tr2_tgt_evt_alias_fl_t *pfn_alias_fl; tr2_tgt_evt_child_start_fl_t *pfn_child_start_fl; tr2_tgt_evt_child_exit_fl_t *pfn_child_exit_fl; + tr2_tgt_evt_child_ready_fl_t *pfn_child_ready_fl; tr2_tgt_evt_thread_start_fl_t *pfn_thread_start_fl; tr2_tgt_evt_thread_exit_fl_t *pfn_thread_exit_fl; tr2_tgt_evt_exec_fl_t *pfn_exec_fl; diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index e3193651f00697..44cf685ea63eed 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -363,6 +363,27 @@ static void fn_child_exit_fl(const char *file, int line, jw_release(&jw); } +static void fn_child_ready_fl(const char *file, int line, + uint64_t us_elapsed_absolute, int cid, int pid, + const char *ready, uint64_t us_elapsed_child) +{ + const char *event_name = "child_ready"; + struct json_writer jw = JSON_WRITER_INIT; + double t_rel = (double)us_elapsed_child / 1000000.0; + + jw_object_begin(&jw, 0); + event_fmt_prepare(event_name, file, line, NULL, &jw); + jw_object_intmax(&jw, "child_id", cid); + jw_object_intmax(&jw, "pid", pid); + jw_object_string(&jw, "ready", ready); + jw_object_double(&jw, "t_rel", 6, t_rel); + jw_end(&jw); + + tr2_dst_write_line(&tr2dst_event, &jw.json); + + jw_release(&jw); +} + static void fn_thread_start_fl(const char *file, int line, uint64_t us_elapsed_absolute) { @@ -589,6 +610,7 @@ struct tr2_tgt tr2_tgt_event = { fn_alias_fl, fn_child_start_fl, fn_child_exit_fl, + fn_child_ready_fl, fn_thread_start_fl, fn_thread_exit_fl, fn_exec_fl, diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index 31b602c171fc69..d66d2b01001712 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -233,6 +233,19 @@ static void fn_child_exit_fl(const char *file, int line, strbuf_release(&buf_payload); } +static void fn_child_ready_fl(const char *file, int line, + uint64_t us_elapsed_absolute, int cid, int pid, + const char *ready, uint64_t us_elapsed_child) +{ + struct strbuf buf_payload = STRBUF_INIT; + double elapsed = (double)us_elapsed_child / 1000000.0; + + strbuf_addf(&buf_payload, "child_ready[%d] pid:%d ready:%s elapsed:%.6f", + cid, pid, ready, elapsed); + normal_io_write_fl(file, line, &buf_payload); + strbuf_release(&buf_payload); +} + static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute, int exec_id, const char *exe, const char **argv) { @@ -311,6 +324,7 @@ struct tr2_tgt tr2_tgt_normal = { fn_alias_fl, fn_child_start_fl, fn_child_exit_fl, + fn_child_ready_fl, NULL, /* thread_start */ NULL, /* thread_exit */ fn_exec_fl, diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index a8018f18cc87e8..e0050c45f1bb0c 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -345,6 +345,20 @@ static void fn_child_exit_fl(const char *file, int line, strbuf_release(&buf_payload); } +static void fn_child_ready_fl(const char *file, int line, + uint64_t us_elapsed_absolute, int cid, int pid, + const char *ready, uint64_t us_elapsed_child) +{ + const char *event_name = "child_ready"; + struct strbuf buf_payload = STRBUF_INIT; + + strbuf_addf(&buf_payload, "[ch%d] pid:%d ready:%s", cid, pid, ready); + + perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute, + &us_elapsed_child, NULL, &buf_payload); + strbuf_release(&buf_payload); +} + static void fn_thread_start_fl(const char *file, int line, uint64_t us_elapsed_absolute) { @@ -537,6 +551,7 @@ struct tr2_tgt tr2_tgt_perf = { fn_alias_fl, fn_child_start_fl, fn_child_exit_fl, + fn_child_ready_fl, fn_thread_start_fl, fn_thread_exit_fl, fn_exec_fl, diff --git a/unpack-trees.c b/unpack-trees.c index 4d26a18f9757f3..0175cdb5152b9f 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1837,6 +1837,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o->result.fsmonitor_last_update = xstrdup_or_null(o->src_index->fsmonitor_last_update); + o->result.fsmonitor_has_run_once = o->src_index->fsmonitor_has_run_once; /* * Sparse checkout loop #1: set NEW_SKIP_WORKTREE on existing entries