Re: [PATCH v2] fsmonitor OSX: fix hangs for submodules

From: Koji Nakamaru <koji.nakamaru@gree.net>
Date: Oct 3, 2024, 08:54 UTC
Message-ID: <CAOTNsDwSTO_iXqG7-ez0O0Y-xbDbEBa6-EiAL-DL=PR1nbM6Zw@mail.gmail.com>
In-Reply-To: <xmqqploimkx1.fsf@gitster.g>

On Thu, Oct 3, 2024 at 3:14 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

>> On windows, there is no process group so the test cannot run
>> correctly. As hangs corrected with the patch occur only for darwin, I
>> would like to skip MINGW in the test. Is it okay?
>
> Surely.  But can we do so without spelling MINGW or WINDOWS out?
>
> That is, if your test requires process group features available, can
> we come up with a lazy prerequisite and use that to decide if we
> skip the test?

I tweaked fsm-listen-win32.c to cause hangs and tested on windows. I'm sorry that simply saying "there is no process group" was not quite correct. Each mingw process has a process group and its win32 subprocesses can be killed by "kill -- -$pgid"

For example, when a hang occurs, the following processes remain.

      PID    PPID    PGID     WINPID   TTY         UID    STIME
        COMMAND
    56782   40923   56782       9484  pty0     1052296 16:23:22
        /mingw64/bin/git
        # mingw git process
    78100       0       0      12564  ?              0 16:23:23
        C:\git-sdk-64\mingw64\libexec\git-core\git.exe
        # win32 process
    86108       0       0      20572  ?              0 16:23:23
        C:\git-sdk-64\mingw64\libexec\git-core\git.exe
        # win32 subprocess
    73328       0       0       7792  ?              0 16:23:23
        C:\git-sdk-64\mingw64\libexec\git-core\git.exe
        # win32 fsmonitor

Show 6 quoted lines

> Earlier in the discussion, you said who are left behind if we did so
> on systems with process groups, but I wonder what happens when we
> throw a signal at the top-level "git" process on Windows, and if it
> behaves better, perhaps we can implement stop_git differently where
> process groups are not available, instead of skipping the tests
> altogether?

If we do "kill 56782" or "kill -- -56782" for the above example, most of processes are terminated except the win32 fsmonitor. This is because the win32 fsmonitor is detached by FreeConsole().

I also tried "git fsmonitor--daemon stop". It was able to communicate with the win32 fsmonitor and the internal status of the win32 fsmonitor changed, but the win32 daemon didn't terminate.

Because it's getting complicated, how about the following:

* specify MINGW
* note in the commit log:
  The test is disabled for MINGW because hangs treated with this patch
  occur only for Darwin and there is no simple way to terminate the
  win32 fsmonitor daemon that hangs.

Koji Nakamaru