[PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

View: New views
7 Messages — Rating Filter:   Alert me  

[PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Bob Denny :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

See
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2407949

[[[
Specific fix for issue #2580 on Win32: Don't do instant kill on tunnel

* subversion/libsvn_ra_svn/client.c
  (make_tunnel): Let tunnel exit normally
]]]

I hope this can make it into 1.7.

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409838
--- subversion-1.6.6rc\subversion\libsvn_ra_svn\client.c Thu Oct 15 21:50:48 2009 UTC
+++ svn-work\subversion\libsvn_ra_svn\client.c Wed Oct 21 15:10:41 2009 UTC
@@ -488,8 +488,24 @@
    * See also the long dicussion in issue #2580 if you really
    * want to know various reasons for these problems and
    * the different opinions on this issue.
+   *
+   * On Win32, APR does not support KILL_ONLY_ONCE. It only has
+   * KILL_ALWAYS and KILL_NEVER. Other modes are converted to
+   * KILL_ALWAYS, which immediately calls TerminateProcess().
+   * This instantly kills the tunnel, leaving sshd and svnserve
+   * on a remote machine running indefinitely. These processes
+   * accumulate. The problem is most often seen with a fast client
+   * machine and a modest internet connection, as the tunnel
+   * is killed before being able to gracefully complete the
+   * session. In that case, svn is unusable 100% of the time on
+   * the windows machine. Thus, on Win32, we use KILL_NEVER and
+   * take the lesser of two evils.
    */
+#ifdef WIN32
+  apr_pool_note_subprocess(pool, proc, APR_KILL_NEVER);
+#else
   apr_pool_note_subprocess(pool, proc, APR_KILL_ONLY_ONCE);
+#endif
 
   /* APR pipe objects inherit by default.  But we don't want the
    * tunnel agent's pipes held open by future child processes

RE: [PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Bob Denny :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Has this been discussed so it can be applied and committed?

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2412599

Re: [PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Gavin Baumanis-4 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Ping. This submission has received no comments.

Gavin


On 22/10/2009, at 02:33 , Bob Denny wrote:

> See
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2407949
>
> [[[
> Specific fix for issue #2580 on Win32: Don't do instant kill on tunnel
>
> * subversion/libsvn_ra_svn/client.c
>  (make_tunnel): Let tunnel exit normally
> ]]]
>
> I hope this can make it into 1.7.
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409838--- 
>  subversion-1.6.6rc\subversion\libsvn_ra_svn\client.c Thu Oct 15  
> 21:50:48 2009 UTC
> +++ svn-work\subversion\libsvn_ra_svn\client.c Wed Oct 21 15:10:41  
> 2009 UTC
> @@ -488,8 +488,24 @@
>    * See also the long dicussion in issue #2580 if you really
>    * want to know various reasons for these problems and
>    * the different opinions on this issue.
> +   *
> +   * On Win32, APR does not support KILL_ONLY_ONCE. It only has
> +   * KILL_ALWAYS and KILL_NEVER. Other modes are converted to
> +   * KILL_ALWAYS, which immediately calls TerminateProcess().
> +   * This instantly kills the tunnel, leaving sshd and svnserve
> +   * on a remote machine running indefinitely. These processes
> +   * accumulate. The problem is most often seen with a fast client
> +   * machine and a modest internet connection, as the tunnel
> +   * is killed before being able to gracefully complete the
> +   * session. In that case, svn is unusable 100% of the time on
> +   * the windows machine. Thus, on Win32, we use KILL_NEVER and
> +   * take the lesser of two evils.
>    */
> +#ifdef WIN32
> +  apr_pool_note_subprocess(pool, proc, APR_KILL_NEVER);
> +#else
>   apr_pool_note_subprocess(pool, proc, APR_KILL_ONLY_ONCE);
> +#endif
>
>   /* APR pipe objects inherit by default.  But we don't want the
>    * tunnel agent's pipes held open by future child processes

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414931

Parent Message unknown Re: [PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Bob Denny :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Thank you Gavin, I pinged it last week as well. It was discussed at length in a
separate thread, and it appeared to be concluded that it was a valid and useful
patch. But no one has committed it as yet...

  -- Bob

> Ping. This submission has received no comments.
>
> Gavin

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414947

Re: [PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Stefan Sperling-7 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Thu, Nov 05, 2009 at 04:56:53PM -0700, Bob Denny wrote:
> Thank you Gavin, I pinged it last week as well. It was discussed at length in a
> separate thread, and it appeared to be concluded that it was a valid and useful
> patch. But no one has committed it as yet...

I talked about this to Bert at Subconf. He had no objections so
I will commit it today. Thanks for the reminder.

Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415048

Re: [PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Stefan Sperling-7 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Fri, Nov 06, 2009 at 10:52:38AM +0100, Stefan Sperling wrote:
> On Thu, Nov 05, 2009 at 04:56:53PM -0700, Bob Denny wrote:
> > Thank you Gavin, I pinged it last week as well. It was discussed at length in a
> > separate thread, and it appeared to be concluded that it was a valid and useful
> > patch. But no one has committed it as yet...
>
> I talked about this to Bert at Subconf. He had no objections so
> I will commit it today. Thanks for the reminder.

r40400

Thanks for the patch!
Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415075

RE: Re: [PATCH] Prevent Win32 tunnels from leaving orphan server processes on remote

by Bob Denny :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

> I talked about this to Bert at Subconf.
> He had no objections so I will commit it
> today. Thanks for the reminder.

Thank you Stefan! I truly appreciate it, and I am glad to have been able to contribute in my small way.

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2419791