[fuchsia] spawn child processes off main thread

As there are conditions under which spawning a child process could take an
extended period of time, it is desirable to perform the spawn off of the main
thread, so as to allow IO to continue (including SSH keepalives).

One such repro trigger is to run `k ut -r 100 brwlock` while trying to start
a new command via ssh. This will enter fdio_spawn on sshd's main thread,
which blocks on the debugcommand service in svchost for the duration of the
tests. The end result is that the ssh client times out due to lack of
response to keepalive, and once the tests finish, sshd will shutdown in
response to the client closing it's connection. The in-flight child process
most often then exits with ZX_ERR_PEER_CLOSED from it's loader service
channel, because sshd was hosting that channel, but has exited (which is all
essentially correct behavior). Other mechanisms to trigger these conditions
can be created, such as loading very large objects from blobfs from slow
storage backends or with slow or throttled CPUs. One such reproduction has
been demonstrated with very large Chrome based test binaries that blocked
appmgr in VmoFromFilenameAt, which was ultimately blocked by blobfs in
FileGetBuffer taking multiple seconds to read, decompress and verify the
blob.

This change does not address the root cause of these problems (synchronous
operations in critical system services), but it mitigates the user-observable
behavior of ssh connection loss during such events, at least for the process
spawning path which is commonly reported.

Additionally, instead of creating a new loader service for each child
process, we re-use a single loader service as configured during the async
loop initialization.

Test: ensure that spawns with and without tty work
Test: ensure that blocked process spawns do not cause connection loss
Bug: 34399
Change-Id: I496c59b19d4e1c16a079e96fdac95a939308e842
diff --git a/fuchsia/fuchsia-compat.c b/fuchsia/fuchsia-compat.c
index 45442a6..25d60d8 100644
--- a/fuchsia/fuchsia-compat.c
+++ b/fuchsia/fuchsia-compat.c
@@ -4,7 +4,6 @@
 
 #include <assert.h>
 #include <fcntl.h>
-#include <pthread.h>
 #include <pwd.h>
 #include <signal.h>
 #include <stdio.h>
@@ -13,6 +12,11 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#ifndef _ALL_SOURCE
+#define _ALL_SOURCE  // Enables thrd_create_with_name in <threads.h>.
+#endif
+#include <threads.h>
+
 #include <fuchsia/hardware/pty/c/fidl.h>
 #include <lib/async-loop/default.h>
 #include <lib/async-loop/loop.h>
@@ -28,9 +32,15 @@
 
 #include "misc.h"
 #include "openbsd-compat/bsd-misc.h"
+#include "xmalloc.h"
 
 static async_loop_t* loop;
 static thrd_t loop_thread;
+static loader_service_t* ldsvc;
+
+async_dispatcher_t* get_async() {
+	return async_loop_get_dispatcher(loop);
+}
 
 void fuchsia_init_async(void) {
 	zx_status_t status;
@@ -45,10 +55,11 @@
 		fprintf(stderr, "fatal: failed to create async loop\n");
 		exit(1);
 	}
-}
 
-async_dispatcher_t* get_async() {
-	return async_loop_get_dispatcher(loop);
+	if ((status = loader_service_create_fs(get_async(), &ldsvc)) != ZX_OK) {
+		fprintf(stderr, "failed to create loader service: %s\n", zx_status_get_string(status));
+		exit(1);
+	}
 }
 
 int chroot(const char* path) { return -1; }
@@ -91,7 +102,6 @@
 
 typedef struct {
 	enum { UNUSED, RUNNING, STOPPED } state;
-	zx_handle_t handle;
 	int exit_code;
 } Child;
 
@@ -117,36 +127,80 @@
 
 static volatile mysig_t sigchld_handler = SIG_IGN;
 
-static void* wait_thread_func(void* voidp) {
-	pthread_detach(pthread_self());
+typedef struct {
+	pid_t pid;
+	zx_handle_t in, out, err;
+	char *command;
+	char **env;
+} LaunchRequest;
 
-	Child* child = voidp;
-
-	zx_signals_t observed;
-	zx_object_wait_one(child->handle, ZX_PROCESS_TERMINATED, ZX_TIME_INFINITE, &observed);
-
-	zx_info_process_t info;
-	size_t actual;
-	zx_object_get_info(child->handle, ZX_INFO_PROCESS, &info, sizeof(info), &actual, NULL);
-
-	child->state = STOPPED;
-	child->exit_code = info.return_code;
-
-	mysig_t handler = sigchld_handler;
-	if (handler == SIG_IGN || handler == SIG_DFL) {
-		// Don't call a handler
+// make_launch_request copies command, but takes ownership over env.
+LaunchRequest* make_launch_request(const char* command, char** env, int in, int out, int err) {
+	LaunchRequest* lr = xmalloc(sizeof(LaunchRequest));
+	lr->pid = get_unused_pid();
+	if (command == NULL) {
+		lr->command = NULL;
 	} else {
-		handler(SIGCHLD);
+		lr->command = xmalloc(strlen(command) + 1);
+		strcpy(lr->command, command);
 	}
 
-	zx_handle_close(child->handle);
-	child->handle = ZX_HANDLE_INVALID;
+	lr->env = env;
 
-	return NULL;
+ /*
+  * ssh's session.c will close out our file descriptors, as it is expecting a
+  * fork() to occur, but we'd otherwise be consuming these fd's "some time
+  * later" in the child thread. We need to consume them now, so we just
+  * decompose them into their handles now. Sadly this repeats some of the work
+  * that fdio_spawn would otherwise be doing for us.
+  */
+	zx_status_t status;
+	if (in == out) {
+		if ((status = fdio_fd_clone(in, &lr->in)) != ZX_OK) {
+			fprintf(stderr, "failed to clone fd %d\n", in);
+		}
+	} else {
+		if ((status = fdio_fd_transfer(in, &lr->in)) != ZX_OK) {
+			fprintf(stderr, "failed to transfer fd %d\n", in);
+		}
+	}
+	if (out == err) {
+		if ((status = fdio_fd_clone(out, &lr->out)) != ZX_OK) {
+			fprintf(stderr, "failed to clone fd %d\n", out);
+		}
+	} else {
+		if ((status = fdio_fd_transfer(out, &lr->out)) != ZX_OK) {
+			fprintf(stderr, "failed to transfer fd %d\n", out);
+		}
+	}
+	if ((status = fdio_fd_transfer(err, &lr->err)) != ZX_OK) {
+		fprintf(stderr, "failed to transfer fd %d\n", err);
+	}
+	return lr;
 }
 
-// Note: **env is consumed by this function and will be freed.
-pid_t fuchsia_launch_child(const char* command, char** env, int in, int out, int err) {
+// free_env free's a **env as produced by session.c
+void free_env(char **env) {
+	char **envf = env;
+	while(*envf != NULL) {
+		free(*envf++);
+	}
+	free(env);
+}
+
+// free_launch_request frees a LaunchRequest. It frees command and env. It does
+// not modify child, and does not close any of the handles.
+void free_launch_request(LaunchRequest* lr) {
+	free(lr->command);
+	free_env(lr->env);
+	free(lr);
+}
+
+static zx_status_t spawn_launch_request(LaunchRequest* lr, zx_handle_t* proc, char *err_msg) {
+	const char* command = lr->command;
+	char** env = lr->env;
+
+	// TODO(CF-578): replace most of this with a "shell runner" instead
 	const char* argv[ARGV_MAX];
 	int argc = 1;
 	argv[0] = "/boot/bin/sh";
@@ -158,21 +212,12 @@
 	}
 	argv[argc] = NULL;
 
-	// TODO(CF-578): replace most of this with a "shell runner" instead
 	zx_status_t status;
-	loader_service_t *ldsvc;
-	if ((status = loader_service_create_fs(get_async(), &ldsvc)) != ZX_OK) {
-		fprintf(stderr, "failed to create loader service: %s\n", zx_status_get_string(status));
-		exit(1);
-	}
-
 	zx_handle_t ldsvc_hnd;
 	if ((status = loader_service_connect(ldsvc, &ldsvc_hnd)) != ZX_OK) {
 		fprintf(stderr, "failed to connect to loader service: %s\n", zx_status_get_string(status));
-		loader_service_release(ldsvc);
 		exit(1);
 	}
-	loader_service_release(ldsvc);
 
 	fdio_spawn_action_t actions[4] = {
 		{
@@ -180,33 +225,33 @@
 				.h = {.id = PA_LDSVC_LOADER, .handle = ldsvc_hnd},
 		},
 		{
-				.action = (in == out) ? FDIO_SPAWN_ACTION_CLONE_FD
-															: FDIO_SPAWN_ACTION_TRANSFER_FD,
-				.fd = {.local_fd = in, .target_fd = STDIN_FILENO},
+				.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
+				.h = {.id = PA_HND(PA_FD, STDIN_FILENO), .handle = lr->in},
 		},
 		{
-				.action = (out == err) ? FDIO_SPAWN_ACTION_CLONE_FD
-																: FDIO_SPAWN_ACTION_TRANSFER_FD,
-				.fd = {.local_fd = out, .target_fd = STDOUT_FILENO},
+				.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
+				.h = {.id = PA_HND(PA_FD, STDOUT_FILENO), .handle = lr->out},
 		},
 		{
-				.action = FDIO_SPAWN_ACTION_TRANSFER_FD,
-				.fd = {.local_fd = err, .target_fd = STDERR_FILENO},
+				.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
+				.h = {.id = PA_HND(PA_FD, STDERR_FILENO), .handle = lr->err},
 		},
 	};
 
-	zx_handle_t proc = 0;
+	uint32_t flags = FDIO_SPAWN_CLONE_JOB | FDIO_SPAWN_CLONE_NAMESPACE;
+	return fdio_spawn_etc(ZX_HANDLE_INVALID, flags, argv[0], argv, (const char* const *)env, 4, actions, proc, err_msg);
+}
+
+static int child_thread_func(void* voidp) {
+	thrd_detach(thrd_current());
+
+	Child* child = get_child(((LaunchRequest*)voidp)->pid);
+	zx_handle_t proc;
+
 	char err_msg[FDIO_SPAWN_ERR_MSG_MAX_LENGTH];
 
-	uint32_t flags = FDIO_SPAWN_CLONE_JOB | FDIO_SPAWN_CLONE_NAMESPACE;
-	status = fdio_spawn_etc(ZX_HANDLE_INVALID, flags, argv[0], argv, (const char* const *)env, 4, actions, &proc, err_msg);
-	// env is constructed in session.c by child_set_env that always printf's new values and callocs the list pointer.
-	// walk each member, freeing them, then free the list.
-	char **envf = env;
-	while(*envf != NULL) {
-		free(*envf++);
-	}
-	free(env);
+	zx_status_t status = spawn_launch_request(voidp, &proc, err_msg);
+	free_launch_request(voidp);
 
 	if (status < 0) {
 		fprintf(stderr, "error from fdio_spawn_etc: %s\n", err_msg);
@@ -214,14 +259,36 @@
 		exit(1);
 	}
 
-	pid_t pid = get_unused_pid();
-	Child* child = get_child(pid);
-	child->state = RUNNING;
-	child->handle = proc;
+	zx_signals_t observed;
+	zx_object_wait_one(proc, ZX_PROCESS_TERMINATED, ZX_TIME_INFINITE, &observed);
 
-	pthread_t wait_thread;
-	if (pthread_create(&wait_thread, NULL, wait_thread_func, (void*)child) != 0) {
-		fprintf(stderr, "Failed to create process waiter thread: %s\n", strerror(errno));
+	zx_info_process_t info;
+	size_t actual;
+	zx_object_get_info(proc, ZX_INFO_PROCESS, &info, sizeof(info), &actual, NULL);
+
+	child->state = STOPPED;
+	child->exit_code = info.return_code;
+
+	mysig_t handler = sigchld_handler;
+	if (handler == SIG_IGN || handler == SIG_DFL) {
+		// Don't call a handler
+	} else {
+		handler(SIGCHLD);
+	}
+
+	zx_handle_close(proc);
+	return 0;
+}
+
+// Note: **env is consumed by this function and will be freed.
+pid_t fuchsia_launch_child(const char* command, char** env, int in, int out, int err) {
+	LaunchRequest* lr = make_launch_request(command, env, in, out, err);
+	pid_t pid = lr->pid;
+	get_child(pid)->state = RUNNING;
+
+	thrd_t child_thread;
+	if (thrd_create_with_name(&child_thread, child_thread_func, (void*)lr, "child-waiter") != 0) {
+		fprintf(stderr, "Failed to create process spawn thread: %s\n", strerror(errno));
 		exit(1);
 	}