[PATCH 0/2] alternate approach to fixing fsmonitor hangs

[Jeff King <peff@peff.net>]

On Mon, Oct 07, 2024 at 06:45:22PM +0900, Koji Nakamaru wrote:

> > -     pthread_mutex_lock(&server_data->work_available_mutex);
> > +     /* If we haven't started yet, we are already holding lock. */
> > +     if (!server_data->started)
> > +             pthread_mutex_lock(&server_data->work_available_mutex);
> >
> >       server_data->shutdown_requested = 1;
> 
> Is this condition inverted?

Yes, good catch.

> > I had also previously checked my suggested solution. So I do think
> > either is a valid solution to the problem.
> 
> I also tested your approach on Windows with a few additions to
> ipc-win32.c, and it worked correctly.

Yeah, I never even tried building mine on Windows, and was just testing
via tmate on our macOS CI environment. :-/

I've added in the necessary Windows bits, along with smoothing a few
rough edges. Especially with the extra Windows changes (which I mostly
had to guess-and-check by pushing to CI), I'm beginning to wonder if my
solution isn't getting a bit too complicated, and maybe yours was the
right way after all.

But I've cleaned it up for presentation here, so at least we can look at
the final form of both and see which we prefer.

  [1/2]: simple-ipc: split async server initialization and running
  [2/2]: fsmonitor: initialize fs event listener before accepting clients

 builtin/fsmonitor--daemon.c          |  6 ++--
 compat/fsmonitor/fsm-listen-darwin.c |  6 ++++
 compat/fsmonitor/fsm-listen-win32.c  |  6 ++++
 compat/simple-ipc/ipc-shared.c       |  5 +--
 compat/simple-ipc/ipc-unix-socket.c  | 28 +++++++++++++---
 compat/simple-ipc/ipc-win32.c        | 48 +++++++++++++++++++++++++---
 simple-ipc.h                         | 17 +++++++---
 7 files changed, 98 insertions(+), 18 deletions(-)

-Peff