Commit
git client had exit with 'read error' some time, if connected git-daemon on windows. The cause was that git-daemon close socket immediately. I fixed this problem. Here is detail the explanation. https://github.com/toshiyuki-ogawa/msysgit/wiki/A-consideration-of--'fatal:-read-error-(invalid-argument)' Signed-off-by: Toshiyuki Ogawa <Toshiyuki.Ogawa.2.71@gmail.com>
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| git-close-socket(1) | ||
| =================== | ||
|
|
||
| NAME | ||
| ---- | ||
| git-close-socket - Close socket like Linux. | ||
|
|
||
|
|
||
| SYNOPSIS | ||
| -------- | ||
| [verse] | ||
| 'git close-socket' | ||
|
|
||
| DESCRIPTION | ||
| ----------- | ||
|
|
||
| Emulate to close socket as Linux. Some system, especialy windows, will close | ||
| soket immediately. In such system git daemon will not communicate some git | ||
| clients well. | ||
| This command resovle the problem. Command will sleep some secondes and exit. | ||
| Sleep seconds is specified below order. | ||
|
|
||
| Windows:: | ||
| 1. 'git-config --int win.sock.time.wait' | ||
| 2. half of registry value 'HKLM\\CurrentControlSet\\Services\\Tcpip\\Parameters\\TcpTimedWaitDelay' | ||
| 3. 240 | ||
|
|
||
|
|
||
| SEE ALSO | ||
| -------- | ||
| http://www.softlab.ntua.gr/facilities/documentation/unix/unix-socket-faq/unix-socket-faq-4.html#ss4.2 | ||
| http://msdn.microsoft.com/en-us/library/windows/desktop/ms737582.asp | ||
| linkgit:git-config[1] | ||
|
|
||
| GIT | ||
| --- | ||
| Part of the linkgit:git[1] suite |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| #include "cache.h" | ||
| #include <time.h> | ||
| #ifdef WIN32 | ||
| #include "winsock-utils.h" | ||
| #endif | ||
|
|
||
| #ifdef EMULATE_TIME_WAIT_SOCKET | ||
| #ifdef WIN32 | ||
| static int wait_for_time_out(void) | ||
| { | ||
| time_t start_time; | ||
| time_t wait_time; | ||
| start_time = time(NULL); | ||
| wait_time = get_socket_time_wait(); | ||
| while (time(NULL) - start_time < wait_time) { | ||
| sleep(1); | ||
| } | ||
| return 0; | ||
| } | ||
| #endif | ||
| #else | ||
| static int wait_for_time_out(void) | ||
| { | ||
| return 0; | ||
| } | ||
| #endif | ||
| int main(int argc, char** argv) | ||
| { | ||
| int result; | ||
|
|
||
| result = wait_for_time_out(); | ||
| return result; | ||
| } |
6 comments
on commit 48f7556
There was a problem hiding this comment.
This is a real big commit, and the subsequent commits provide some fixups.
If you don't mind, I would like to have you test a different tack first (I cannot test it because I do not encounter the read error myself): how about calling setsockopt() with SO_LINGER? It might fix the problem without many changes.
There was a problem hiding this comment.
I would be going to test again. But I don't understand "a different track". How do I test on a different track?
Off course I did try Calling "setsockopt() with SO_LINGER" on first, but it did not result what I want.
The calling "setsockopt" affect on the process. It did not work after the process dead. I need to keep opening daemon side socket after the git-upload-pack process dead.
There was a problem hiding this comment.
@toshiyuki-ogawa okay, just making sure.
The big problem I see with this commit is that it is really big. It took me quite a while to read through it, and I did not even have time to review it.
It seems that it introduces quite a few new files with quite a lot of functions that I would not deem necessary for emulating a TIME_WAIT. Besides, I really wonder why we have to linger around when we clearly do not read/write anything. Maybe we first have to make sure that the client received everything and that it is safe to close the socket now?
In any case, the final form of this pull request needs to have small, reviewable commits, and it also cannot introduce #ifdef WIN32 in the non-platform-specific code (it still does so).
So I guess the most pressing thing I would like to know: how does the git daemon know that all of its data was sent to the client, i.e. that it is safe to close the socket now?
There was a problem hiding this comment.
Were I in your place, I think this commit is big as you do. I am going to answer some questions about this commit and modify some lines which don't keep git rules.
The most important thing is that git client exit without 'read error' in any communication environment. I don't persist this commit but strongly hope to fix 'read error' bug.
I think that emulating TIME_WAIT is more natural than another ways. In git, some small processes archive a user request. In my option, this is git software design. I want to keep the style. When I was fixing this problem, I had have an another fixing way which was to call 'setsockopt' and make git-daemon wait until child process dead. But I felt that it didn't keep the style. I expected that it was more complex modification of daemon.c than this commit. If child process inherit socket attributes, this bug fix will be simpler than this commit.
In my opinion git-daemon would have no way to know that client received all data. Whereas git-upload-pack flush all data on exit. At that time all data is in server side socket buffer. I think the buffer is in kernel on unix like os, on the other hand it is in process on windows. In windows, the buffer is disposed on process exit. In unix like os, the buffer remain in kernel for few minutes. I think unix like os is optimistic way that client will recieve all data in few minutes after server side process exit, Windows os way is designed for multi-thread environment in single process.
I have two options about this pull request.
First one, to keep git rules, write some functions platform-specific code on compat/mingw.c and remove '#ifdef WIN32' from non-platform-specific code.
Second one, How about to create a branch to check 'read error', try calling setsockopt linger and resolve 'read error' step by step? This branch has some tags which are represent check points. If you approve of this proposal, I am going to create the branch. It will take some weeks to pull request, because I have one or two hours every week to work on the branch.
There was a problem hiding this comment.
@toshiyuki-ogawa thank you for your detailed answer!
About the time constraints: I hear you. I myself cannot spend more than 15 minutes per day on average (i.e. about the same time you can spare) for working on Git.
Maybe a better way to tackle this would be to write a simple pair of programs communicating with each other through sockets; one which listens for a connection, accepts immediately, sends one single byte and exits right away. The other connecting to the port, sleeping one second, then reading one byte, then print that out and exit. That should demonstrate the differences between Unix and Windows without the overhead of all the Git stuff.
There was a problem hiding this comment.
Okay, so I burnt up an hour of my Git time budget to come up with such a minimal example. However, I was not even able to get it to run without the sleep because for some reason, connect() always errors out. See my feeble attempt here.
If you have time to work on it, I would very much appreciate it. Otherwise I'll probably pick it up next week.
should be
[--shutdown-out-socket]