From f03d2ec3e6a940410f78019d022e35bf08fc9d2f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 8 Oct 2024 04:33:47 -0400 Subject: [PATCH] simple-ipc: split async server initialization and running To start an async ipc server, you call ipc_server_run_async(). That initializes the ipc_server_data object, and starts all of the threads running, which may immediately start serving clients. This can create some awkward timing problems, though. In the fsmonitor daemon (the sole user of the simple-ipc system), we want to create the ipc server early in the process, which means we may start serving clients before the rest of the daemon is fully initialized. To solve this, let's break run_async() into two parts: an initialization which allocates all data and spawns the threads (without letting them run), and a start function which actually lets them begin work. Since we have two simple-ipc implementations, we have to handle this twice: - in ipc-unix-socket.c, we have a central listener thread which hands connections off to worker threads using a work_available mutex. We can hold that mutex after init, and release it when we're ready to start. We do need an extra "started" flag so that we know whether the main thread is holding the mutex or not (e.g., if we prematurely stop the server, we want to make sure all of the worker threads are released to hear about the shutdown). - in ipc-win32.c, we don't have a central mutex. So we'll introduce a new startup_barrier mutex, which we'll similarly hold until we're ready to let the threads proceed. We again need a "started" flag here to make sure that we release the barrier mutex when shutting down, so that the sub-threads can proceed to the finish. I've renamed the run_async() function to init_async() to make sure we catch all callers, since they'll now need to call the matching start_async(). We could leave run_async() as a wrapper that does both, but there's not much point. There are only two callers, one of which is fsmonitor, which will want to actually do work between the two calls. And the other is just a test-tool wrapper. For now I've added the start_async() calls in fsmonitor where they would otherwise have happened, so there should be no behavior change with this patch. Signed-off-by: Jeff King Acked-by: Koji Nakamaru Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index f3f6bd330b9ec9..4a077810d0e112 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -1215,6 +1215,8 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) _("could not start IPC thread pool on '%s'"), state->path_ipc.buf); + ipc_server_start_async(&state->ipc_server_data); + /* * Start the fsmonitor listener thread to collect filesystem * events.