Skip to content

Commit

Permalink
simple-ipc: split async server initialization and running
Browse files Browse the repository at this point in the history
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 <peff@peff.net>
Acked-by: Koji Nakamaru <koji.nakamaru@gree.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
peff authored and Git for Windows Build Agent committed Nov 1, 2024
1 parent cc406db commit f03d2ec
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions builtin/fsmonitor--daemon.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit f03d2ec

Please sign in to comment.