Run server with child process.

[toshiyuki-ogawa]

You can run server with child process.
You can specify some server options.
Here is options
1 -l [number] you can specify [number] how many lines to send message for client
2 -p You can send server message in child process which is spawned in server
I got bellow error in my environment, if I run server to write data in child process file descriptor.
Here is instructions and out put
./server 1234 -l 256 -p & ./client 127.0.0.1 1234
Here is output
0 I got the connection
1 I got the connection
2 I got the connection
3 I got the connection
4 I goError reading from socket:Function not implemented

@dscho Would you mean that I work for above extension, not yourself? If you didn't, I will cancel this pull request.

[buildhive]

MSysGit - the development behind Git for Windows » git #195 SUCCESS
This pull request looks good
(what's this?)

[dscho]

@toshiyuki-ogawa the error message you receive is not the same as with git-daemon, correct?

Maybe it would be better if I was able to reproduce the original problem with git daemon. I still would like to shrink down your pull request #70 without breaking your fix...

[toshiyuki-ogawa]

the error message you receive is not the same as with git-daemon, correct?

I wouldn't think error message is not same unfortunately.

reading from socket:Function not implemented

The above message was produced from reading the socket which is connected to server child process stdout.
I think the above message come from error_win_to_posix.
In [https://github.com/msysgit/git/blob/socket-test/client.c], errno is not mapped collect message that the reason why client produce the message 'Function not implemented'.

[dscho]

Bummer.

Do you think you could come up with a patch to, say, git receive-pack that triggers the error with the git daemon?

Please understand that my reluctance stems from the huge diff at https://github.com/msysgit/git/pull/70/files just to fix something that intuitively should add a Sleep() call somewhere in the code path. And that latter solution -- while hacky -- would have a lot less opportunity to hide an accidental bug or security issue.

[toshiyuki-ogawa]

Do you think you could come up with a patch to, say, git receive-pack that triggers the error with the git daemon?

It is difficult to answer your question. I can't say whether yes or no.
I think this sample(client and server) would lead a collect answer.

I would like to focus on this test(client and server),
At least, we can understand that server child process didn't handle the file descriptor which is socket .
Could we go forward from this truth?

[dscho]

@toshiyuki-ogawa TBH I am unsure how to proceed. My idea to have a minimal example did not work out: we do not have a robust way to reproduce the problem you tried to fix in #70. I hope to find some time this weekend to play more with it, but feel free to go whichever direction you feel makes most sense.

[toshiyuki-ogawa]

My idea to have a minimal example did not work out

I don't think so. Your minimal example reproduced the problem in #70.
Here is evidence:
'Error reading from socket:Function not implemented'

The message was different from git client, but it was same problem that the client did not completely read data which server send.
I think that the different of message come from code of git-index-pack.
In this example I can't help doubting to inherit socket between server and child process.
In this example, The problem was produced when server create child process and leave socket to it.
figure 1. No error message case

1 client ----> server

2 client <==> server (write data to socket)
                      [server exit]
3 client <----

4 client [read all data completely]

figure 2. Error message case

1 client ----> server
                [create process] * leave socket to clt_writer
2 client ----> server  ==  clt_writer
                 [server exit]           
3 client <===========> clt_writer (clt_writer write data to stdout.)
                               [clt_writer exit]
4 client <------------------

5 client 'Error reading from socket:Function not implemented'

I think the remarkable points are 2 and 3 in above figure 2. If the clt_writer stdout has socket attribute, client may read data with out error message.

[dscho]

Thanks for the detailed explanation!

I'll give it a try but for now I am starting to be optimistic again that the issue might be fixed simply by refusing to exit() from the server as long as there is even one running child...

[toshiyuki-ogawa]

I think that it would be fine to make git by refusing to exit. I want git version to run in any communication environment.

I would concern myself about bellow two topics.

1. To sleep for an interval
   If you take a way to sleep for an interval, How long do git-daemon sleep? If you have hard coded interval constants, is your code reviewable?
2. To wait for child process exit
   If you take a way to wait for child process, you have to make not only child processes but also ground child processes and great ground child processes. Some executable module may run by another git programs. Do you want to make their wait? I think that this way may cause another side effect.

Actually, In my pull-request #70, All processes which run from git-daemon need to be modified to check whether stdout and stdin are treated as socket.

I checked the issue #101 recently, and I studied about the issue. I am doubting the problem of socket attributes inheritance too. I added some lines to debug in some codes, and found a line which never come back. The line is only writing to some data to file descriptor. Here is the line which never come back.
[https://github.com/msysgit/git/blob/master/csum-file.c#L29]

Here is calling sequence

$> git push
     |
     +builtin/push.c
       |
       +transport.c
         |
         +send-pack.c (* In this file, call start_command, the child process will not take care socket.)
           |
           +buitin/pack-objects.c
             |
             +csum-file.c

I would like to study and fix the problem, but I am afraid of unacceptable pull-request for complex code.

[dscho]

Since Git for Windows 1.x was retired, I am closing this ticket. However, it will be very valuable when I come around to attacking git-for-windows#304.