[PATCH v2] builtin/credential-cache--daemon: fix error when "exit"ing on Cygwin

Clients can signal the git-credential-cache(1) daemon that it is supposed to exit by sending it an "exit" command. The details around how exactly the daemon exits seem to be rather intricate as spelt out by a comment surrounding our call to exit(3p), as we need to be mindful around closing the client streams before we signal the client.

The logic is broken on Cygwin though: when a client asks the daemon to exit, they won't see the EOF and will instead get an error message:

  fatal: read error from cache daemon: Software caused connection abort

This issue is known in Cygwin for quite a while already [1] [2] and seems to come from a change in Cygwin itself. While we haven't figured out what exact change that was, the effect is that we see ECONNABORTED now instead of ECONNRESET when trying to read from the socket connected to the daemon when the daemon has shut down. It is somewhat dubious that errno changed in this way: read(3p) does not mention ECONNABORTED as a possible error code, so the behaviour seems to not be POSIX-compliant.

We have discussed a couple of workarounds:

  - Explicitly close file streams when handling the "exit" action in
    `serve_one_client()`. This may easily lead to races because we need
    to be very careful about the order in which we delete sockets.

  - We can adapt `serve_one_client()` to not use exit(3) anymore, and
    instead handle the shutdown in the outer loop. This allows us to
    handle the shutdown elsewhere and be less intimate with the calling
    context.

  - Start handling ECONNABORTED.

The last option seems like it is both the easiest and least-risky fix. It is unlikely that it would negatively affect any other platforms given that read(3p) shouldn't even set the code in the first place, and it fixes the issue on Cygwin.

Fix the issue by handling both ECONNRESET and ECONNABORTED.

[1]: https://github.com/cygporter/git/issues/51 [2]: https://lore.kernel.org/git/9dc3e85f-a532-6cff-de11-1dfb2e4bc6b6@ramsayjones.plus.com/

Original-patch-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/credential-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)