{"thread":{"id":"36702","subject":"[PATCH/RFC] send-pack.c: Allow to disable side-band-64k","startedAt":"2014-05-19T19:07:14Z","lastAt":"2014-05-20T09:01:31Z","messageCount":8,"participants":["Thomas Braun","Jonathan Nieder","Erik Faye-Lund"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"242194","messageId":"1400526434-3132-1-git-send-email-thomas.braun@byte-physics.de","threadId":"36702","inReplyTo":null,"subject":"[PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Thomas Braun","fromEmail":"thomas.braun@byte-physics.de","sentAt":"2014-05-19T19:07:14Z","receivedAt":"2014-05-19T19:07:14Z","isPatch":true,"sender":{"key":"thomas.braun@byte-physics.de","avatar":null},"body":"Since commit 0c499ea60f the send-pack builtin uses the side-band-64k\ncapability if advertised by the server. Unfortunately this breaks\npushing over the dump git protocol with a windows git client.\n\nThe detailed reasons for this breakage are (by courtesy of Jeff\nPreshing, quoted from\nhttps://groups.google.com/d/msg/msysgit/at8D7J-h7mw/eaLujILGUWoJ):\n----------------------------------------------------------------------------\nMinGW wraps Windows sockets in CRT file descriptors in order to mimic\nthe functionality of POSIX sockets. This causes msvcrt.dll to treat\nsockets as Installable File System (IFS) handles, calling ReadFile,\nWriteFile, DuplicateHandle and CloseHandle on them. This approach works\nwell in simple cases on recent versions of Windows, but does not support\nall usage patterns. In particular, using this approach, any attempt to\nread & write concurrently on the same socket (from one or more\nprocesses) will deadlock in a scenario where the read waits for a\nresponse from the server which is only invoked after the write. This is\nwhat send_pack currently attempts to do in the use_sideband codepath.\n----------------------------------------------------------------------------\n\nThe new config option \"sendpack.sideband\" allows to override the\nside-band-64k capability of the server.\n\nOther transportation methods like ssh and http/https still benefit from\nthe sideband channel, therefore the default value of \"sendpack.sideband\"\nis still true.\n\nAlternative approaches considered but deemed too invasive:\n- Rewrite read/write wrappers in mingw.c in order to distinguish between\n a file descriptor which has a socket behind and a file descriptor\n which has a file behind.\n- Turning the capability side-band-64k off completely. This would remove a useful\n feature for users of non-affected transport protocols.\n\nSigned-off-by: Thomas Braun \n---\n\nThis patch, with a slightly less polished commit message, is already part of\nmsysgit/git see b68e386. \n\nA lengthy discussion can be found here [1].\n\nWhat do you think, is this also for you as upstream interesting?\n\n[1]: https://github.com/msysgit/git/issues/101\n\n Documentation/config.txt | 6 ++++++\n send-pack.c | 14 +++++++++++++-\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1932e9b..13ff657 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2435,3 +2435,9 @@ web.browser::\n \tSpecify a web browser that may be used by some commands.\n \tCurrently only linkgit:git-instaweb[1] and linkgit:git-help[1]\n \tmay use it.\n+\n+sendpack.sideband::\n+ Allows to disable the side-band-64k capability for send-pack even\n+ when it is advertised by the server. Makes it possible to work\n+ around a limitation in the git for windows implementation together\n+ with the dump git protocol. Defaults to true.\ndiff --git a/send-pack.c b/send-pack.c\nindex 6129b0f..aace1fc 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -12,6 +12,16 @@\n #include \"version.h\"\n #include \"sha1-array.h\"\n \n+static int config_use_sideband = 1;\n+\n+static int send_pack_config(const char *var, const char *value, void *unused)\n+{\n+\tif (!strcmp(\"sendpack.sideband\", var))\n+\t\tconfig_use_sideband = git_config_bool(var, value);\n+\n+\treturn 0;\n+}\n+\n static int feed_object(const unsigned char *sha1, int fd, int negative)\n {\n \tchar buf[42];\n@@ -209,6 +219,8 @@ int send_pack(struct send_pack_args *args,\n \tint ret;\n \tstruct async demux;\n \n+\tgit_config(send_pack_config, NULL);\n+\n \t/* Does the other end support the reporting? */\n \tif (server_supports(\"report-status\"))\n \t\tstatus_report = 1;\n@@ -216,7 +228,7 @@ int send_pack(struct send_pack_args *args,\n \t\tallow_deleting_refs = 1;\n \tif (server_supports(\"ofs-delta\"))\n \t\targs->use_ofs_delta = 1;\n-\tif (server_supports(\"side-band-64k\"))\n+\tif (config_use_sideband && server_supports(\"side-band-64k\"))\n \t\tuse_sideband = 1;\n \tif (server_supports(\"quiet\"))\n \t\tquiet_supported = 1;\n-- \n1.9.1\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242197","messageId":"20140519193340.GP12314@google.com","threadId":"36702","inReplyTo":"1400526434-3132-1-git-send-email-thomas.braun@byte-physics.de","subject":"Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-05-19T19:33:40Z","receivedAt":"2014-05-19T19:33:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nThomas Braun wrote:\n\n> pushing over the dump git protocol with a windows git client.\n\nI've never heard of the dump git protocol. Do you mean the git\nprotocol that's used with git:// URLs?\n\n[...]\n> Alternative approaches considered but deemed too invasive:\n> - Rewrite read/write wrappers in mingw.c in order to distinguish between\n> a file descriptor which has a socket behind and a file descriptor\n> which has a file behind.\n\nI assume here \"too invasive\" means \"too much engineering effort\"?\n\nIt sounds like a clean fix, not too invasive at all. But I can\nunderstand wanting a stopgap in the meantime.\n\n> - Turning the capability side-band-64k off completely. This would remove a useful\n> feature for users of non-affected transport protocols.\n\nWould it make sense to turn off sideband unconditionally on Windows\nwhen using the relevant protocols?\n\nI assume someone on the list wouldn't mind writing such a patch, so I\ndon't think the engineering effort would be a problem for that.\n\nThanks,\nJonathan\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242199","messageId":"CABPQNSbYVZP4CcatWLuv5m-RFWWgOuKOJJcceBkcEzyJQQ+BqA@mail.gmail.com","threadId":"36702","inReplyTo":"20140519193340.GP12314@google.com","subject":"Re: Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-05-19T20:00:57Z","receivedAt":"2014-05-19T20:00:57Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder wrote:\n> Hi,\n>\n> Thomas Braun wrote:\n>\n>> pushing over the dump git protocol with a windows git client.\n>\n> I've never heard of the dump git protocol. Do you mean the git\n> protocol that's used with git:// URLs?\n>\n> [...]\n>> Alternative approaches considered but deemed too invasive:\n>> - Rewrite read/write wrappers in mingw.c in order to distinguish between\n>> a file descriptor which has a socket behind and a file descriptor\n>> which has a file behind.\n>\n> I assume here \"too invasive\" means \"too much engineering effort\"?\n>\n> It sounds like a clean fix, not too invasive at all. But I can\n> understand wanting a stopgap in the meantime.\n>\n\nYeah, now that the problem seems to be understood, I don't think that\nwould be too bad. I recently killed off our previous write()-wrapper\nin c9df6f4, but I see no reason why we can't add a new one.\n\nWould we need to wrap both ends, shouldn't wrapping only reading be\ngood enough to prevent deadlocking?\n\ncompat/poll/poll.c already contains a function called IsSocketHandle\nthat is able to tell if a HANDLE points to a socket or not.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242200","messageId":"CABPQNSb7B6D_XthOOCfadXA8agLUhoSYy5=20OL2fh598evR1g@mail.gmail.com","threadId":"36702","inReplyTo":"CABPQNSbYVZP4CcatWLuv5m-RFWWgOuKOJJcceBkcEzyJQQ+BqA@mail.gmail.com","subject":"Re: Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-05-19T20:29:59Z","receivedAt":"2014-05-19T20:29:59Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, May 19, 2014 at 10:00 PM, Erik Faye-Lund wrote:\n> On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder wrote:\n>> Hi,\n>>\n>> Thomas Braun wrote:\n>>\n>>> pushing over the dump git protocol with a windows git client.\n>>\n>> I've never heard of the dump git protocol. Do you mean the git\n>> protocol that's used with git:// URLs?\n>>\n>> [...]\n>>> Alternative approaches considered but deemed too invasive:\n>>> - Rewrite read/write wrappers in mingw.c in order to distinguish between\n>>> a file descriptor which has a socket behind and a file descriptor\n>>> which has a file behind.\n>>\n>> I assume here \"too invasive\" means \"too much engineering effort\"?\n>>\n>> It sounds like a clean fix, not too invasive at all. But I can\n>> understand wanting a stopgap in the meantime.\n>>\n>\n> Yeah, now that the problem seems to be understood, I don't think that\n> would be too bad. I recently killed off our previous write()-wrapper\n> in c9df6f4, but I see no reason why we can't add a new one.\n>\n> Would we need to wrap both ends, shouldn't wrapping only reading be\n> good enough to prevent deadlocking?\n>\n> compat/poll/poll.c already contains a function called IsSocketHandle\n> that is able to tell if a HANDLE points to a socket or not.\n\nThis very quick attempt did not work out :(\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 0335958..ec1d81f 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)\n return fd;\n }\n\n+#define is_console_handle(h) (((long) (h) & 3) == 3)\n+\n+static int is_socket_handle(HANDLE h)\n+{\n+ WSANETWORKEVENTS ev;\n+\n+ if (is_console_handle(h))\n+ return 0;\n+\n+ /*\n+ * Under Wine, it seems that getsockopt returns 0 for pipes too.\n+ * WSAEnumNetworkEvents instead distinguishes the two correctly.\n+ */\n+ ev.lNetworkEvents = 0xDEADBEEF;\n+ WSAEnumNetworkEvents((SOCKET) h, NULL, &ev);\n+ return ev.lNetworkEvents != 0xDEADBEEF;\n+}\n+\n+#undef read\n+ssize_t mingw_read(int fd, void *buf, size_t count)\n+{\n+ int ret;\n+ HANDLE fh = (HANDLE)_get_osfhandle(fd);\n+\n+ if (fh == INVALID_HANDLE_VALUE) {\n+ errno = EBADF;\n+ return -1;\n+ }\n+\n+ if (!is_socket_handle(fh))\n+ return read(fd, buf, count);\n+\n+ ret = recv((SOCKET)fh, buf, count, 0);\n+ if (ret < 0)\n+ errno = WSAGetLastError();\n+ return ret;\n+}\n+\n+#undef write\n+ssize_t mingw_write(int fd, const void *buf, size_t count)\n+{\n+ int ret;\n+ HANDLE fh = (HANDLE)_get_osfhandle(fd);\n+\n+ if (fh == INVALID_HANDLE_VALUE) {\n+ errno = EBADF;\n+ return -1;\n+ }\n+\n+ if (!is_socket_handle(fh))\n+ return write(fd, buf, count);\n+\n+ return send((SOCKET)fh, buf, count, 0);\n+ if (ret < 0)\n+ errno = WSAGetLastError();\n+ return ret;\n+}\n+\n+\n static BOOL WINAPI ctrl_ignore(DWORD type)\n {\n return TRUE;\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 08b83fe..1690098 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n\n+ssize_t mingw_read(int fd, void *buf, size_t count);\n+#define read mingw_read\n+\n+ssize_t mingw_write(int fd, const void *buf, size_t count);\n+#define write mingw_write\n+\n int mingw_fgetc(FILE *stream);\n #define fgetc mingw_fgetc\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242203","messageId":"537A7454.6030001@byte-physics.de","threadId":"36702","inReplyTo":"20140519193340.GP12314@google.com","subject":"Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Thomas Braun","fromEmail":"thomas.braun@byte-physics.de","sentAt":"2014-05-19T21:15:00Z","receivedAt":"2014-05-19T21:15:00Z","isPatch":true,"sender":{"key":"thomas.braun@byte-physics.de","avatar":null},"body":"Am 19.05.2014 21:33, schrieb Jonathan Nieder:\n> Hi,\n>\n> Thomas Braun wrote:\n>\n>> pushing over the dump git protocol with a windows git client.\n>\n> I've never heard of the dump git protocol. Do you mean the git\n> protocol that's used with git:// URLs?\n\nYou are right I mean the protocol involving git:// URLs. But \nunfortunately I got it wrong as according to [1] the git:// is one of \nthe so-called smart protocols. That was also the source where I read \nthat there are smart and dump protocols.\n\n[1]: http://git-scm.com/book/en/Git-Internals-Transfer-Protocols\n\n> [...]\n>> Alternative approaches considered but deemed too invasive:\n>> - Rewrite read/write wrappers in mingw.c in order to distinguish between\n>> a file descriptor which has a socket behind and a file descriptor\n>> which has a file behind.\n>\n> I assume here \"too invasive\" means \"too much engineering effort\"?\n>\n> It sounds like a clean fix, not too invasive at all. But I can\n> understand wanting a stopgap in the meantime.\n\nNo actually I meant too invasive in the sense of \"requiring large \nrewrites which only benefit git on windows and hurt all others\".\n\nThe two fixes I can think of either involve:\n- In a read *and* write wrapper the need to check if the fd is a socket, \nif yes use send/recv if no use read/write. According to Erik's comments \nthis should be possible. But I would deem the expected performance \npenalty quite large as that will be done in every call.\n- Rewriting read/write to accept windows handles instead of file \ndescriptors. Only a theoretical option IMHO.\n\nFor me the goal is also to minimise the diff between git and msysgit/git.\n\n>\n>> - Turning the capability side-band-64k off completely. This would remove a useful\n>> feature for users of non-affected transport protocols.\n>\n> Would it make sense to turn off sideband unconditionally on Windows\n> when using the relevant protocols?\n>\n\nYes, if this would be also acceptable for git.git.\n\nI can check at the call site of send_pack in transport.c what protocol \nis in use, and then pass a new parameter use_sideband to it.\nOr maybe \"adapt\" server_capabilities in connect.c to not include \nside-band-64k if using git:// ?\n\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242204","messageId":"CABPQNSbyOzxyv91t87tJidwhdY=LJ5HZu=+vyETAFMkOFV6_HQ@mail.gmail.com","threadId":"36702","inReplyTo":"537A7454.6030001@byte-physics.de","subject":"Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-05-19T21:20:06Z","receivedAt":"2014-05-19T21:20:06Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, May 19, 2014 at 11:15 PM, Thomas Braun\n wrote:\n> Am 19.05.2014 21:33, schrieb Jonathan Nieder:\n>\n>> Hi,\n>>\n>> Thomas Braun wrote:\n>>\n>>> pushing over the dump git protocol with a windows git client.\n>>\n>>\n>> I've never heard of the dump git protocol. Do you mean the git\n>> protocol that's used with git:// URLs?\n>\n>\n> You are right I mean the protocol involving git:// URLs. But unfortunately I\n> got it wrong as according to [1] the git:// is one of the so-called smart\n> protocols. That was also the source where I read that there are smart and\n> dump protocols.\n>\n> [1]: http://git-scm.com/book/en/Git-Internals-Transfer-Protocols\n>\n>\n>> [...]\n>>>\n>>> Alternative approaches considered but deemed too invasive:\n>>> - Rewrite read/write wrappers in mingw.c in order to distinguish between\n>>> a file descriptor which has a socket behind and a file descriptor\n>>> which has a file behind.\n>>\n>>\n>> I assume here \"too invasive\" means \"too much engineering effort\"?\n>>\n>> It sounds like a clean fix, not too invasive at all. But I can\n>> understand wanting a stopgap in the meantime.\n>\n>\n> No actually I meant too invasive in the sense of \"requiring large rewrites\n> which only benefit git on windows and hurt all others\".\n>\n> The two fixes I can think of either involve:\n> - In a read *and* write wrapper the need to check if the fd is a socket, if\n> yes use send/recv if no use read/write. According to Erik's comments this\n> should be possible. But I would deem the expected performance penalty quite\n> large as that will be done in every call.\n\nYou clearly haven't stepped through MSVCRT's read and write implementations :P\n\nI wouldn't worry too much about this, at least not until the numbers are in.\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242237","messageId":"537B164F.3080606@byte-physics.de","threadId":"36702","inReplyTo":"CABPQNSb7B6D_XthOOCfadXA8agLUhoSYy5=20OL2fh598evR1g@mail.gmail.com","subject":"Re: Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Thomas Braun","fromEmail":"thomas.braun@byte-physics.de","sentAt":"2014-05-20T08:46:07Z","receivedAt":"2014-05-20T08:46:07Z","isPatch":true,"sender":{"key":"thomas.braun@byte-physics.de","avatar":null},"body":"Am 19.05.2014 22:29, schrieb Erik Faye-Lund:\n>> [...]\n>> Would we need to wrap both ends, shouldn't wrapping only reading be\n>> good enough to prevent deadlocking?\n>>\n>> compat/poll/poll.c already contains a function called IsSocketHandle\n>> that is able to tell if a HANDLE points to a socket or not.\n>\n> This very quick attempt did not work out :(\n>\n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 0335958..ec1d81f 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)\n> return fd;\n> }\n>\n> +#define is_console_handle(h) (((long) (h) & 3) == 3)\n> +\n> +static int is_socket_handle(HANDLE h)\n> +{\n> + WSANETWORKEVENTS ev;\n> +\n> + if (is_console_handle(h))\n> + return 0;\n> +\n> + /*\n> + * Under Wine, it seems that getsockopt returns 0 for pipes too.\n> + * WSAEnumNetworkEvents instead distinguishes the two correctly.\n> + */\n> + ev.lNetworkEvents = 0xDEADBEEF;\n> + WSAEnumNetworkEvents((SOCKET) h, NULL, &ev);\n> + return ev.lNetworkEvents != 0xDEADBEEF;\n> +}\n> +\n> +#undef read\n> +ssize_t mingw_read(int fd, void *buf, size_t count)\n> +{\n> + int ret;\n> + HANDLE fh = (HANDLE)_get_osfhandle(fd);\n> +\n> + if (fh == INVALID_HANDLE_VALUE) {\n> + errno = EBADF;\n> + return -1;\n> + }\n> +\n> + if (!is_socket_handle(fh))\n> + return read(fd, buf, count);\n> +\n> + ret = recv((SOCKET)fh, buf, count, 0);\n> + if (ret < 0)\n> + errno = WSAGetLastError();\n> + return ret;\n> +}\n> +\n> +#undef write\n> +ssize_t mingw_write(int fd, const void *buf, size_t count)\n> +{\n> + int ret;\n> + HANDLE fh = (HANDLE)_get_osfhandle(fd);\n> +\n> + if (fh == INVALID_HANDLE_VALUE) {\n> + errno = EBADF;\n> + return -1;\n> + }\n> +\n> + if (!is_socket_handle(fh))\n> + return write(fd, buf, count);\n> +\n> + return send((SOCKET)fh, buf, count, 0);\n> + if (ret < 0)\n> + errno = WSAGetLastError();\n> + return ret;\n> +}\n> +\n> +\n> static BOOL WINAPI ctrl_ignore(DWORD type)\n> {\n> return TRUE;\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 08b83fe..1690098 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);\n> int mingw_open (const char *filename, int oflags, ...);\n> #define open mingw_open\n>\n> +ssize_t mingw_read(int fd, void *buf, size_t count);\n> +#define read mingw_read\n> +\n> +ssize_t mingw_write(int fd, const void *buf, size_t count);\n> +#define write mingw_write\n> +\n> int mingw_fgetc(FILE *stream);\n> #define fgetc mingw_fgetc\n\nAccording to [1] you also have to pass WSA_FLAG_OVERLAPPED to avoid the deadlock.\n\nWith that change I don't get a hang anymore but a read error with \nerrno 10054 aka WSAECONNRESET.\n\n[1]: https://groups.google.com/forum/#!msg/msysgit/at8D7J-h7mw/PM9w-d41cDYJ\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 383cafe..4a58135 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -366,6 +366,71 @@ int mingw_open (const char *filename, int oflags, ...)\n \treturn fd;\n }\n \n+#define is_console_handle(h) (((long) (h) & 3) == 3)\n+\n+static int is_socket_handle(HANDLE h)\n+{\n+ WSANETWORKEVENTS ev;\n+\n+ if (is_console_handle(h))\n+ return 0;\n+\n+ /*\n+ * Under Wine, it seems that getsockopt returns 0 for pipes too.\n+ * WSAEnumNetworkEvents instead distinguishes the two correctly.\n+ */\n+ ev.lNetworkEvents = 0xDEADBEEF;\n+ WSAEnumNetworkEvents((SOCKET) h, NULL, &ev);\n+ return ev.lNetworkEvents != 0xDEADBEEF;\n+}\n+\n+#undef read\n+ssize_t mingw_read(int fd, void *buf, size_t count)\n+{\n+ int ret;\n+ HANDLE fh = (HANDLE)_get_osfhandle(fd);\n+\n+ if (fh == INVALID_HANDLE_VALUE) {\n+ errno = EBADF;\n+ return -1;\n+ }\n+\n+ if (!is_socket_handle(fh))\n+ return read(fd, buf, count);\n+\n+ ret = recv((SOCKET)fh, buf, count, 0);\n+ if (ret < 0)\n+ {\n+ errno = WSAGetLastError();\n+ warning(\"errno %d\",errno);\n+ }\n+ return ret;\n+}\n+\n+#undef write\n+ssize_t mingw_write(int fd, const void *buf, size_t count)\n+{\n+ int ret;\n+ HANDLE fh = (HANDLE)_get_osfhandle(fd);\n+\n+ if (fh == INVALID_HANDLE_VALUE) {\n+ errno = EBADF;\n+ return -1;\n+ }\n+\n+ if (!is_socket_handle(fh))\n+ return write(fd, buf, count);\n+\n+ ret = send((SOCKET)fh, buf, count, 0);\n+ if (ret < 0)\n+ {\n+ errno = WSAGetLastError();\n+ warning(\"errno %d\",errno);\n+ }\n+ return ret;\n+}\n+\n+\n static BOOL WINAPI ctrl_ignore(DWORD type)\n {\n \treturn TRUE;\n@@ -1542,7 +1607,7 @@ int mingw_socket(int domain, int type, int protocol)\n \tSOCKET s;\n \n \tensure_socket_initialization();\n-\ts = WSASocket(domain, type, protocol, NULL, 0, 0);\n+\ts = WSASocket(domain, type, protocol, NULL, 0, WSA_FLAG_OVERLAPPED);\n \tif (s == INVALID_SOCKET) {\n \t\t/*\n \t\t * WSAGetLastError() values are regular BSD error codes\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 08b83fe..1690098 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n+ssize_t mingw_read(int fd, void *buf, size_t count);\n+#define read mingw_read\n+\n+ssize_t mingw_write(int fd, const void *buf, size_t count);\n+#define write mingw_write\n+\n int mingw_fgetc(FILE *stream);\n #define fgetc mingw_fgetc\n \n\n\n\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"242238","messageId":"CABPQNSaa3gRLY0A73-RLfwSy7K7gXHpS8QwFbxxYSCwVyi82Yg@mail.gmail.com","threadId":"36702","inReplyTo":"537B164F.3080606@byte-physics.de","subject":"Re: Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-05-20T09:01:31Z","receivedAt":"2014-05-20T09:01:31Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, May 20, 2014 at 10:46 AM, Thomas Braun\n wrote:\n> Am 19.05.2014 22:29, schrieb Erik Faye-Lund:\n>>> [...]\n>>> Would we need to wrap both ends, shouldn't wrapping only reading be\n>>> good enough to prevent deadlocking?\n>>>\n>>> compat/poll/poll.c already contains a function called IsSocketHandle\n>>> that is able to tell if a HANDLE points to a socket or not.\n>>\n>> This very quick attempt did not work out :(\n>>\n>> diff --git a/compat/mingw.c b/compat/mingw.c\n>> index 0335958..ec1d81f 100644\n>> --- a/compat/mingw.c\n>> +++ b/compat/mingw.c\n>> @@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)\n>> return fd;\n>> }\n>>\n>> +#define is_console_handle(h) (((long) (h) & 3) == 3)\n>> +\n>> +static int is_socket_handle(HANDLE h)\n>> +{\n>> + WSANETWORKEVENTS ev;\n>> +\n>> + if (is_console_handle(h))\n>> + return 0;\n>> +\n>> + /*\n>> + * Under Wine, it seems that getsockopt returns 0 for pipes too.\n>> + * WSAEnumNetworkEvents instead distinguishes the two correctly.\n>> + */\n>> + ev.lNetworkEvents = 0xDEADBEEF;\n>> + WSAEnumNetworkEvents((SOCKET) h, NULL, &ev);\n>> + return ev.lNetworkEvents != 0xDEADBEEF;\n>> +}\n>> +\n>> +#undef read\n>> +ssize_t mingw_read(int fd, void *buf, size_t count)\n>> +{\n>> + int ret;\n>> + HANDLE fh = (HANDLE)_get_osfhandle(fd);\n>> +\n>> + if (fh == INVALID_HANDLE_VALUE) {\n>> + errno = EBADF;\n>> + return -1;\n>> + }\n>> +\n>> + if (!is_socket_handle(fh))\n>> + return read(fd, buf, count);\n>> +\n>> + ret = recv((SOCKET)fh, buf, count, 0);\n>> + if (ret < 0)\n>> + errno = WSAGetLastError();\n>> + return ret;\n>> +}\n>> +\n>> +#undef write\n>> +ssize_t mingw_write(int fd, const void *buf, size_t count)\n>> +{\n>> + int ret;\n>> + HANDLE fh = (HANDLE)_get_osfhandle(fd);\n>> +\n>> + if (fh == INVALID_HANDLE_VALUE) {\n>> + errno = EBADF;\n>> + return -1;\n>> + }\n>> +\n>> + if (!is_socket_handle(fh))\n>> + return write(fd, buf, count);\n>> +\n>> + return send((SOCKET)fh, buf, count, 0);\n>> + if (ret < 0)\n>> + errno = WSAGetLastError();\n>> + return ret;\n>> +}\n>> +\n>> +\n>> static BOOL WINAPI ctrl_ignore(DWORD type)\n>> {\n>> return TRUE;\n>> diff --git a/compat/mingw.h b/compat/mingw.h\n>> index 08b83fe..1690098 100644\n>> --- a/compat/mingw.h\n>> +++ b/compat/mingw.h\n>> @@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);\n>> int mingw_open (const char *filename, int oflags, ...);\n>> #define open mingw_open\n>>\n>> +ssize_t mingw_read(int fd, void *buf, size_t count);\n>> +#define read mingw_read\n>> +\n>> +ssize_t mingw_write(int fd, const void *buf, size_t count);\n>> +#define write mingw_write\n>> +\n>> int mingw_fgetc(FILE *stream);\n>> #define fgetc mingw_fgetc\n>\n> According to [1] you also have to pass WSA_FLAG_OVERLAPPED to avoid the deadlock.\n>\n> With that change I don't get a hang anymore but a read error with\n> errno 10054 aka WSAECONNRESET.\n>\n> [1]: https://groups.google.com/forum/#!msg/msysgit/at8D7J-h7mw/PM9w-d41cDYJ\n>\n\nYeah, sorry, I noticed this right after sending and tested with that\nas well. My results were the same :/\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"msysGit\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"}]}