Merge branch 'pt/fsmonitor-linux' into seen

The fsmonitor daemon has been implemented for Linux.

* pt/fsmonitor-linux:
  fsmonitor: close inherited file descriptors and detach in daemon
  run-command: add close_fd_above_stderr option
  fsmonitor: add tests for Linux
  fsmonitor: implement filesystem change listener for Linux
  fsmonitor: deduplicate settings logic for Unix platforms
  fsmonitor: deduplicate IPC path logic for Unix platforms
  fsmonitor: use pthread_cond_timedwait for cookie wait
  compat/win32: add pthread_cond_timedwait
  fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
  fsmonitor: fix khash memory leak in do_handle_client

Author: gitster

Documentation/config/fsmonitor--daemon.adoc

@@ -4,8 +4,8 @@ fsmonitor.allowRemote::
     behavior.  Only respected when `core.fsmonitor` is set to `true`.
 
 fsmonitor.socketDir::
-    This Mac OS-specific option, if set, specifies the directory in
+    This Mac OS and Linux-specific option, if set, specifies the directory in
     which to create the Unix domain socket used for communication
     between the fsmonitor daemon and various Git commands. The directory must
-    reside on a native Mac OS filesystem.  Only respected when `core.fsmonitor`
+    reside on a native filesystem.  Only respected when `core.fsmonitor`
     is set to `true`.

Documentation/git-fsmonitor--daemon.adoc

@@ -76,9 +76,9 @@ repositories; this may be overridden by setting `fsmonitor.allowRemote` to
 correctly with all network-mounted repositories, so such use is considered
 experimental.
 
-On Mac OS, the inter-process communication (IPC) between various Git
+On Mac OS and Linux, the inter-process communication (IPC) between various Git
 commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a
-special type of file -- which is supported by native Mac OS filesystems,
+special type of file -- which is supported by native Mac OS and Linux filesystems,
 but not on network-mounted filesystems, NTFS, or FAT32.  Other filesystems
 may or may not have the needed support; the fsmonitor daemon is not guaranteed
 to work with these filesystems and such use is considered experimental.
@@ -87,13 +87,33 @@ By default, the socket is created in the `.git` directory.  However, if the
 `.git` directory is on a network-mounted filesystem, it will instead be
 created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a
 network-mounted filesystem, in which case you must set the configuration
-variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native
+variable `fsmonitor.socketDir` to the path of a directory on a native
 filesystem in which to create the socket file.
 
 If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`)
-is on a native Mac OS file filesystem the fsmonitor daemon will report an
+is on a native filesystem the fsmonitor daemon will report an
 error that will cause the daemon and the currently running command to exit.
 
+LINUX CAVEATS
+~~~~~~~~~~~~~
+
+On Linux, the fsmonitor daemon uses inotify to monitor filesystem events.
+The inotify system has per-user limits on the number of watches that can
+be created.  The default limit is typically 8192 watches per user.
+
+For large repositories with many directories, you may need to increase
+this limit.  Check the current limit with:
+
+    cat /proc/sys/fs/inotify/max_user_watches
+
+To temporarily increase the limit:
+
+    sudo sysctl fs.inotify.max_user_watches=65536
+
+To make the change permanent, add to `/etc/sysctl.conf`:
+
+    fs.inotify.max_user_watches=65536
+
 CONFIGURATION
 -------------
 

Makefile

@@ -2380,13 +2380,13 @@ 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
-	COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o
 endif
 
 ifdef FSMONITOR_OS_SETTINGS
 	COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS
+	COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_OS_SETTINGS).o
 	COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o
-	COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o
+	COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_DAEMON_BACKEND).o
 endif
 
 ifdef WITH_BREAKING_CHANGES

builtin/fsmonitor--daemon.c

@@ -86,6 +86,8 @@ static int do_as_client__send_stop(void)
 {
 	struct strbuf answer = STRBUF_INIT;
 	int ret;
+	int max_wait_ms = 30000;
+	int elapsed_ms = 0;
 
 	ret = fsmonitor_ipc__send_command("quit", &answer);
 
@@ -96,8 +98,16 @@ static int do_as_client__send_stop(void)
 		return ret;
 
 	trace2_region_enter("fsm_client", "polling-for-daemon-exit", NULL);
-	while (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
+	while (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING) {
+		if (elapsed_ms >= max_wait_ms) {
+			trace2_region_leave("fsm_client",
+					    "polling-for-daemon-exit", NULL);
+			return error(_("daemon did not stop within %d seconds"),
+				     max_wait_ms / 1000);
+		}
 		sleep_millisec(50);
+		elapsed_ms += 50;
+	}
 	trace2_region_leave("fsm_client", "polling-for-daemon-exit", NULL);
 
 	return 0;
@@ -197,20 +207,31 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie(
 	unlink(cookie_pathname.buf);
 
 	/*
-	 * Technically, this is an infinite wait (well, unless another
-	 * thread sends us an abort).  I'd like to change this to
-	 * use `pthread_cond_timedwait()` and return an error/timeout
-	 * and let the caller do the trivial response thing, but we
-	 * don't have that routine in our thread-utils.
-	 *
-	 * After extensive beta testing I'm not really worried about
-	 * this.  Also note that the above open() and unlink() calls
-	 * will cause at least two FS events on that path, so the odds
-	 * of getting stuck are pretty slim.
+	 * Wait for the listener thread to observe the cookie file.
+	 * Time out after a short interval so that the client
+	 * does not hang forever if the filesystem does not deliver
+	 * events (e.g., on certain container/overlay filesystems
+	 * where inotify watches succeed but events never arrive).
 	 */
-	while (cookie->result == FCIR_INIT)
-		pthread_cond_wait(&state->cookies_cond,
-				  &state->main_lock);
+	{
+		struct timeval now;
+		struct timespec ts;
+		int err = 0;
+
+		gettimeofday(&now, NULL);
+		ts.tv_sec = now.tv_sec + 1;
+		ts.tv_nsec = now.tv_usec * 1000;
+
+		while (cookie->result == FCIR_INIT && !err)
+			err = pthread_cond_timedwait(&state->cookies_cond,
+						     &state->main_lock,
+						     &ts);
+		if (err == ETIMEDOUT && cookie->result == FCIR_INIT) {
+			trace_printf_key(&trace_fsmonitor,
+					 "cookie_wait timed out");
+			cookie->result = FCIR_ERROR;
+		}
+	}
 
 done:
 	hashmap_remove(&state->cookies, &cookie->entry, NULL);
@@ -671,7 +692,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 	const struct fsmonitor_batch *batch;
 	struct fsmonitor_batch *remainder = NULL;
 	intmax_t count = 0, duplicates = 0;
-	kh_str_t *shown;
+	kh_str_t *shown = NULL;
 	int hash_ret;
 	int do_trivial = 0;
 	int do_flush = 0;
@@ -909,8 +930,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 		total_response_len += payload.len;
 	}
 
-	kh_release_str(shown);
-
 	pthread_mutex_lock(&state->main_lock);
 
 	if (token_data->client_ref_count > 0)
@@ -954,6 +973,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 	trace2_data_intmax("fsmonitor", the_repository, "response/count/duplicates", duplicates);
 
 cleanup:
+	kh_destroy_str(shown);
 	strbuf_release(&response_token);
 	strbuf_release(&requested_token_id);
 	strbuf_release(&payload);
@@ -1405,6 +1425,7 @@ static int fsmonitor_run_daemon(void)
 done:
 	pthread_cond_destroy(&state.cookies_cond);
 	pthread_mutex_destroy(&state.main_lock);
+	hashmap_clear(&state.cookies);
 	fsm_listen__dtor(&state);
 	fsm_health__dtor(&state);
 
@@ -1420,7 +1441,7 @@ static int fsmonitor_run_daemon(void)
 	return err;
 }
 
-static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED)
+static int try_to_run_foreground_daemon(int detach_console)
 {
 	/*
 	 * Technically, we don't need to probe for an existing daemon
@@ -1440,10 +1461,21 @@ static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED)
 		fflush(stderr);
 	}
 
+	if (detach_console) {
 #ifdef GIT_WINDOWS_NATIVE
-	if (detach_console)
 		FreeConsole();
+#else
+		/*
+		 * Create a new session so that the daemon is detached
+		 * from the parent's process group.  This prevents
+		 * shells with job control (e.g. bash with "set -m")
+		 * from waiting on the daemon when they wait for a
+		 * foreground command that implicitly spawned it.
+		 */
+		if (setsid() == -1)
+			warning_errno(_("setsid failed"));
 #endif
+	}
 
 	return !!fsmonitor_run_daemon();
 }
@@ -1506,6 +1538,7 @@ static int try_to_start_background_daemon(void)
 	cp.no_stdin = 1;
 	cp.no_stdout = 1;
 	cp.no_stderr = 1;
+	cp.close_fd_above_stderr = 1;
 
 	sbgr = start_bg_command(&cp, bg_wait_cb, NULL,
 				fsmonitor__start_timeout_sec);

compat/fsmonitor/fsm-health-linux.c

@@ -0,0 +1,33 @@
+#include "git-compat-util.h"
+#include "config.h"
+#include "fsmonitor-ll.h"
+#include "fsm-health.h"
+#include "fsmonitor--daemon.h"
+
+/*
+ * The Linux fsmonitor implementation uses inotify which has its own
+ * mechanisms for detecting filesystem unmount and other events that
+ * would require the daemon to shutdown.  Therefore, we don't need
+ * a separate health thread like Windows does.
+ *
+ * These stub functions satisfy the interface requirements.
+ */
+
+int fsm_health__ctor(struct fsmonitor_daemon_state *state UNUSED)
+{
+	return 0;
+}
+
+void fsm_health__dtor(struct fsmonitor_daemon_state *state UNUSED)
+{
+	return;
+}
+
+void fsm_health__loop(struct fsmonitor_daemon_state *state UNUSED)
+{
+	return;
+}
+
+void fsm_health__stop_async(struct fsmonitor_daemon_state *state UNUSED)
+{
+}

compat/fsmonitor/fsm-ipc-darwin.c compat/fsmonitor/fsm-ipc-unix.ccompat/fsmonitor/fsm-ipc-darwin.c renamed to compat/fsmonitor/fsm-ipc-unix.c

@@ -27,13 +27,15 @@ const char *fsmonitor_ipc__get_path(struct repository *r)
 	if (ipc_path)
 		return ipc_path;
 
-
 	/* By default the socket file is created in the .git directory */
 	if (fsmonitor__is_fs_remote(r->gitdir) < 1) {
 		ipc_path = fsmonitor_ipc__get_default_path();
 		return ipc_path;
 	}
 
+	if (!r->worktree)
+		BUG("repository has no worktree");
+
 	git_SHA1_Init(&sha1ctx);
 	git_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
 	git_SHA1_Final(hash, &sha1ctx);