Logic error in SSHSessionProcessProtocol.inConnectionLost()

[bshi]

The program gitconnbug.py (attached) wraps git-shell and and can be executed to start the sample server. Run a command like

# The /abspath/to/git/repo should be readable by the server user
$ git clone ssh://user@localhost:2222/abspath/to/git/repo
(password "user")

To test the server. I have encountered an issue that would cause a git client fail in an apparently non-deterministic manner. On my workstation, I am able to reproduce the error at least once every 5 tries. The behavior I see is attached at the end of this email. As far as I can tell, the client fails due to a premature inConnectionLost() call in the SSHSessionProcessProtocol that sends an EOF.

As a workaround, when TraceProcessProtocol.inConnectionLost() is overriden to do nothing, this error goes away.

Successful case (server side logging, timestamp cut out):

[SSHChannel session (0) on SSHService ssh-connection
on SSHServerTransport,1,127.0.0.1] executing command
"git-upload-pack '/Users/bshi/sandbox/poop'"
[-] TPP.outReceived(...) 199 bytes
[-] TPP.outReceived(...) 146 bytes
[-] TPP.outReceived(...) 8 bytes
[-] TPP.outReceived(...) 33 bytes
[-] TPP.outReceived(...) 41 bytes
[-] TPP.outReceived(...) 77 bytes
[-] TPP.outReceived(...) 1228 bytes
[-] TPP.outReceived(...) 8192 bytes
[-] TPP.outReceived(...) 4567 bytes
[-] TPP.inConnectionLost()
[-] sending eof
[-] exitCode: 0

Successful case (client-side shell session):

$ GIT_TRACE=1 git clone ssh://user@localhost:2222/Users/bshi/sandbox/poop

trace: built-in: git 'clone' 'ssh://user@localhost:2222/Users/bshi/sandbox/poop'
Initialized empty Git repository in /tmp/poop/.git/
trace: run_command: 'ssh' '-p' '2222' 'user@localhost' 'git-upload-pack
       '\''/Users/bshi/sandbox/poop'\'''
user@localhost's password:
trace: run_command: 'index-pack' '--stdin' '-v' '--fix-thin'
             '--keep=fetch-pack 763 on Bo-Shis-MacBook-Pro.local'
trace: exec: 'git' 'index-pack' '--stdin' '-v' '--fix-thin'
             '--keep=fetch-pack 763 on Bo-Shis-MacBook-Pro.local'
trace: exec: 'git-index-pack' '--stdin' '-v' '--fix-thin'
             '--keep=fetch-pack 763 on Bo-Shis-MacBook-Pro.local'
trace: run_command: 'git-index-pack' '--stdin' '-v' '--fix-thin'
             '--keep=fetch-pack 763 on Bo-Shis-MacBook-Pro.local'
remote: Counting objects: 50, done.
remote: Compressing objects: 100% (35/35), done.
remote: Total 50 (delta 16), reused 43 (delta 14)
Receiving objects: 100% (50/50), 12.36 KiB, done.
Resolving deltas: 100% (16/16), done.

Failure case (server side):

   [SSHChannel session (0) on SSHService ssh-connection
   on SSHServerTransport,2,127.0.0.1] executing command
   "git-upload-pack '/Users/bshi/sandbox/poop'"
   [-] TPP.outReceived(...) 345 bytes
   [-] TPP.outReceived(...) 8 bytes
   [-] TPP.outReceived(...) 33 bytes
   [-] TPP.outReceived(...) 8192 bytes
   [-] TPP.inConnectionLost()
   [-] sending eof
   [-] TPP.outReceived(...) 5903 bytes
   [-] exitCode: 0
   [-] sending request exit-status
   [-] sending close 0

Failure case ("trace:" messages similar, cut out for readability):

   $ GIT_TRACE=1 git clone ssh://user@localhost:2222/Users/bshi/sandbox/poop
   user@localhost's password:
   ...
   ...
   remote: Counting objects: 50, done.
   fatal: The remote end hung up unexpectedly
   fatal: early EOF
   fatal: index-pack failed

[bshi]

ssh server reproducing issue (port 2222)

[bshi]

Proposed patch based on James' and Michael's comments at

 http://twistedmatrix.com/pipermail/twisted-python/2010-March/021776.html

The patch makes the assumption that outConnectionLost() and errConnectionLost() can only be called once for a given processprotocol instance. If this is not true, I need to reword things a bit.

[therve]

(In [28797]) Branching to 'ssh-session-close-4350'

[therve]

(In [28800]) Merge ssh-session-close-4350

Author: bshi Reviewer: therve Fixes: #4350

Fix a behavior in SSHSessionProcessProtocol, which used to close the session when stdin is closed, and now closes when stdout and stderr are closed.

[therve]

Thanks a lot for the ticket and the patch!