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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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, see for example [1], but the exact root
cause is not known.

As it turns out, we can avoid the issue by explicitly closing the client
streams via fclose(3p). I'm not sure at all where the claimed atexit(3p)
handler mentioned in the comment is supposed to live, but from all I can
see we do not have any installed that would close the sockets for us. So
this leaves me with a bit of a sour taste overall.

That being said, I couldn't spot anything obviously wrong with closing
the streams ourselves, and it does fix the issue on Cygwin without any
regressions on other platforms as far as I can see. So let's go for this
fix, even though I cannot properly explain it.

[1]: https://github.com/cygporter/git/issues/51

Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---

I've Cc'd Adam, who is the maintainer of the Git package in Cygwin, as
well as Peff, who is the original author of the below comment. I'd be
really happy if one of you could enlighten me here :)

Patrick

 builtin/credential-cache--daemon.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
index bc22f5c6d24..5a09df5c167 100644
--- a/builtin/credential-cache--daemon.c
+++ b/builtin/credential-cache--daemon.c
@@ -156,13 +156,11 @@ static void serve_one_client(FILE *in, FILE *out)
 	}
 	else if (!strcmp(action.buf, "exit")) {
 		/*
-		 * It's important that we clean up our socket first, and then
-		 * signal the client only once we have finished the cleanup.
-		 * Calling exit() directly does this, because we clean up in
-		 * our atexit() handler, and then signal the client when our
-		 * process actually ends, which closes the socket and gives
-		 * them EOF.
+		 * We must close our file handles before we exit such that the
+		 * client will receive an EOF.
 		 */
+		fclose(in);
+		fclose(out);
 		exit(0);
 	}
 	else if (!strcmp(action.buf, "erase"))
-- 
2.47.0.72.gef8ce8f3d4.dirty





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux