Bad interaction between svn and OpenSSH auto connection pooling

[Peter Samuelson <pe...@p12n.org>]

Background for the dev@svn list and the openssh maintainers: I'm
commenting on a Debian bug report, http://bugs.debian.org/413102, where
'svn log svn+ssh://...' sometimes encounters a broken pipe due to the
ssh subprocess dying too soon.  I explain below.


[Peter Samuelson]
> It seems that in the 'svn log' case, the session pool, passed into
> [libsvn_ra_svn/client.c] open_session(), is destroyed too early, thus
> killing ssh.  I'll try to investigate further.

OK, svn is not actually buggy.  Here is what happens:

'svn log svn+ssh://...' fires off _two_ ssh processes.  (Erik Huelsmann
explains on IRC that this is intentional, connections are treated as
lightweight, libsvn_ra makes no attempt to reuse connections for
efficiency.)

The first ssh process finishes its business and is explicitly killed
(by apr, when the session pool is destroyed).  The second connection is
still active.

That would be fine ... IF the two ssh processes were independent, which
normally they are.  But not if you use the "ControlMaster auto"
feature!  This causes ssh to do its own connection pooling: the first
ssh process sets itself up as a "master" and the second one detects the
master and becomes a slave.  People use this feature for efficiency,
since setting up and tearing down ssh connections really isn't cheap.
(There's the network protocol and the cryptography, but also password
prompting and such.)

So I'm still trying to think of ideas how to proceed here.  It's really
not svn's fault that launching two ssh processes, then killing the
first, effectively kills the second as well.  On the other hand, it's
not really ssh's fault that a master process is being explicitly killed
while still in use.  (Left on its own, the master would wait until all
its slave connections are closed, before dying.)

Note, one workaround is to explicitly instantiate an ssh connection
pool master, with 'ssh -M', before using 'svn log'.  Another is to set
SVN_SSH='ssh -o ControlMaster=no' (or the equivalent in
~/.subversion/config).  Does anyone have any better ideas, for either a
fix or a workaround?

Thanks,
Peter

Re: Bad interaction between svn and OpenSSH auto connection pooling

[Alan Barrett <ap...@cequrux.com>]

On Sat, 03 Mar 2007, Peter Samuelson wrote:
> > Ideally, svn should close all the relevant file descriptors and wait
> > for ssh to die by itself.  If it doesn't die after a few seconds,
> > kill it with SIGTERM; if it still doesn't die after a few more
> > seconds, only then kill it with SIGKILL.
>
> Setting a few-second timeout might not be trivial from within the
> bowels of a library.  It involves either blocking the whole thread, or
> some sort of alarm and signal handling, with no knowledge of what the
> host application might already be using these facilities for.

I am not familiar with the internals of either APR or svn, but I was
under the impression that some kind of event framework was available,
and that this could be used to program arbitrary sequences of timed
operations.

> > On the ssh side, perhaps the control master should fork and
> > daemonise itself, to insulate it from being killed by svn.
>
> Well ... if you kill a process, doesn't that usually mean you really
> do intend for it to die?  I think ssh is already doing the right thing
> there.

Well, when svn kills ssh, it really intends for one svn-to-svnserve
connection to die; I don't think it intends for other svn-to-svnserve
connections to die, and svn has no way of knowing that the same ssh
process is multiplexing several svn-to-svnserve connections over a
single ssh-to-sshd connection.

When ssh is killed, how will it know whether the killer intended to
terminate only the initial svn-to-svnserve connection, or also other
connections multiplexed though the same "control master"?  So I am
suggesting that ssh should grow new functionality to place the control
master into the background, such that killing the control master kills
all multiplexed connections, but killing the original ssh process kills
only the original conenction.

--apb (Alan Barrett)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bad interaction between svn and OpenSSH auto connection pooling

[Peter Samuelson <pe...@p12n.org>]

[Alan Barrett]
> Ideally, svn should close all the relevant file descriptors and wait
> for ssh to die by itself.  If it doesn't die after a few seconds, kill
> it with SIGTERM; if it still doesn't die after a few more seconds, only
> then kill it with SIGKILL.

Setting a few-second timeout might not be trivial from within the
bowels of a library.  It involves either blocking the whole thread, or
some sort of alarm and signal handling, with no knowledge of what the
host application might already be using these facilities for.

> svn currently kills it immediately with
> SIGKILL, which is very rude, and I call this a bug.

Yeah, http://subversion.tigris.org/issues/show_bug.cgi?id=2580 quotes
the comment that explains the SIGKILL: "Killing the tunnel agent with
SIGTERM leads to unsightly stderr output from ssh."

In Debianland, the ssh maintainers have patched out this "unsightly
stderr output", so I've changed svn's SIGKILL to the more conventional
SIGTERM+sleep+SIGKILL.

> On the ssh side, perhaps the control master should fork and
> daemonise itself, to insulate it from being killed by svn.

Well ... if you kill a process, doesn't that usually mean you really do
intend for it to die?  I think ssh is already doing the right thing
there.

Peter

Re: Bad interaction between svn and OpenSSH auto connection pooling

[Alan Barrett <ap...@cequrux.com>]

On Sat, 03 Mar 2007, Peter Samuelson wrote:
> OK, svn is not actually buggy.  Here is what happens:
>
> 'svn log svn+ssh://...' fires off _two_ ssh processes.  (Erik
> Huelsmann explains on IRC that this is intentional, connections
> are treated as lightweight, libsvn_ra makes no attempt to reuse
> connections for efficiency.)

That might be intentional, but "connections are lightweight" is an
incorrect assumption, and it would be nice if libsvn_ra did not make
this assumption.

> The first ssh process finishes its business and is explicitly killed
> (by apr, when the session pool is destroyed).  The second connection
> is still active.

Ideally, svn should close all the relevant file descriptors and wait
for ssh to die by itself.  If it doesn't die after a few seconds, kill
it with SIGTERM; if it still doesn't die after a few more seconds, only
then kill it with SIGKILL.  svn currently kills it immediately with
SIGKILL, which is very rude, and I call this a bug.  Processes killed
with SIGKILL do not get a chance to clean up after themselves (for
example, delete temporary files), and I had to go to a lot of extra
trouble in wrapper scripts referred to by $SVN_SSH to work around this
problem.

On the ssh side, perhaps the control master should fork and
daemonise itself, to insulate it from being killed by svn.

--apb (Alan Barrett)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bad interaction between svn and OpenSSH auto connection pooling

[Max Bowsher <ma...@ukf.net>]

Erik Huelsmann wrote:
> On 3/3/07, Max Bowsher <ma...@ukf.net> wrote:
>> I wonder why svn kills the ssh processes at all. It seems to me that svn
>> should simply close its pipes to the subprocess, and leave it to the
>> subprocess to determine when to exit.
>>
>> That would solve this problem rather nicely - can anyone see any
>> potential flaws?
> 
> Daniel Berlin did (he read our code):
> http://svn.haxx.se/users/archive-2005-11/0194.shtml

Yes, I'm entirely aware of that comment, but it's far too terse to
actually be helpful. It doesn't really explain more than "this was prone
to weird problems".

Max.

Re: Bad interaction between svn and OpenSSH auto connection pooling

[Erik Huelsmann <eh...@gmail.com>]

On 3/3/07, Max Bowsher <ma...@ukf.net> wrote:
> Peter Samuelson wrote:
> > Background for the dev@svn list and the openssh maintainers: I'm
> > commenting on a Debian bug report, http://bugs.debian.org/413102, where
> > 'svn log svn+ssh://...' sometimes encounters a broken pipe due to the
> > ssh subprocess dying too soon.  I explain below.
> >
> >
> > [Peter Samuelson]
> >> It seems that in the 'svn log' case, the session pool, passed into
> >> [libsvn_ra_svn/client.c] open_session(), is destroyed too early, thus
> >> killing ssh.  I'll try to investigate further.
> >
> > OK, svn is not actually buggy.  Here is what happens:
> >
> > 'svn log svn+ssh://...' fires off _two_ ssh processes.  (Erik Huelsmann
> > explains on IRC that this is intentional, connections are treated as
> > lightweight, libsvn_ra makes no attempt to reuse connections for
> > efficiency.)
> >
> > The first ssh process finishes its business and is explicitly killed
> > (by apr, when the session pool is destroyed).  The second connection is
> > still active.
> >
> > That would be fine ... IF the two ssh processes were independent, which
> > normally they are.  But not if you use the "ControlMaster auto"
> > feature!  This causes ssh to do its own connection pooling: the first
> > ssh process sets itself up as a "master" and the second one detects the
> > master and becomes a slave.  People use this feature for efficiency,
> > since setting up and tearing down ssh connections really isn't cheap.
> > (There's the network protocol and the cryptography, but also password
> > prompting and such.)
> >
> > So I'm still trying to think of ideas how to proceed here.  It's really
> > not svn's fault that launching two ssh processes, then killing the
> > first, effectively kills the second as well.  On the other hand, it's
> > not really ssh's fault that a master process is being explicitly killed
> > while still in use.  (Left on its own, the master would wait until all
> > its slave connections are closed, before dying.)
> >
> > Note, one workaround is to explicitly instantiate an ssh connection
> > pool master, with 'ssh -M', before using 'svn log'.  Another is to set
> > SVN_SSH='ssh -o ControlMaster=no' (or the equivalent in
> > ~/.subversion/config).  Does anyone have any better ideas, for either a
> > fix or a workaround?
>
> I wonder why svn kills the ssh processes at all. It seems to me that svn
> should simply close its pipes to the subprocess, and leave it to the
> subprocess to determine when to exit.
>
> That would solve this problem rather nicely - can anyone see any
> potential flaws?

Daniel Berlin did (he read our code):
http://svn.haxx.se/users/archive-2005-11/0194.shtml

> Obviously, there's the chance that a buggy tunnel agent might not exit,
>  but really, that's the fault of the agent, not of Subversion, so I
> don't see that the Subversion client needs to care about that,
> especially not at the cost of inhibiting this OpenSSH feature.

Maybe the problems indicated in dannyb's post don't relate to modern
ssh versions anymore?

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Bad interaction between svn and OpenSSH auto connection pooling

[Max Bowsher <ma...@ukf.net>]

Peter Samuelson wrote:
> Background for the dev@svn list and the openssh maintainers: I'm
> commenting on a Debian bug report, http://bugs.debian.org/413102, where
> 'svn log svn+ssh://...' sometimes encounters a broken pipe due to the
> ssh subprocess dying too soon.  I explain below.
> 
> 
> [Peter Samuelson]
>> It seems that in the 'svn log' case, the session pool, passed into
>> [libsvn_ra_svn/client.c] open_session(), is destroyed too early, thus
>> killing ssh.  I'll try to investigate further.
> 
> OK, svn is not actually buggy.  Here is what happens:
> 
> 'svn log svn+ssh://...' fires off _two_ ssh processes.  (Erik Huelsmann
> explains on IRC that this is intentional, connections are treated as
> lightweight, libsvn_ra makes no attempt to reuse connections for
> efficiency.)
> 
> The first ssh process finishes its business and is explicitly killed
> (by apr, when the session pool is destroyed).  The second connection is
> still active.
> 
> That would be fine ... IF the two ssh processes were independent, which
> normally they are.  But not if you use the "ControlMaster auto"
> feature!  This causes ssh to do its own connection pooling: the first
> ssh process sets itself up as a "master" and the second one detects the
> master and becomes a slave.  People use this feature for efficiency,
> since setting up and tearing down ssh connections really isn't cheap.
> (There's the network protocol and the cryptography, but also password
> prompting and such.)
> 
> So I'm still trying to think of ideas how to proceed here.  It's really
> not svn's fault that launching two ssh processes, then killing the
> first, effectively kills the second as well.  On the other hand, it's
> not really ssh's fault that a master process is being explicitly killed
> while still in use.  (Left on its own, the master would wait until all
> its slave connections are closed, before dying.)
> 
> Note, one workaround is to explicitly instantiate an ssh connection
> pool master, with 'ssh -M', before using 'svn log'.  Another is to set
> SVN_SSH='ssh -o ControlMaster=no' (or the equivalent in
> ~/.subversion/config).  Does anyone have any better ideas, for either a
> fix or a workaround?

I wonder why svn kills the ssh processes at all. It seems to me that svn
should simply close its pipes to the subprocess, and leave it to the
subprocess to determine when to exit.

That would solve this problem rather nicely - can anyone see any
potential flaws?

Obviously, there's the chance that a buggy tunnel agent might not exit,
 but really, that's the fault of the agent, not of Subversion, so I
don't see that the Subversion client needs to care about that,
especially not at the cost of inhibiting this OpenSSH feature.

Max.