You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Gavin Baumanis <ga...@thespidernet.com> on 2009/05/01 10:55:02 UTC

Re: [PATCH] (updated) fix for ssh zombies introduced with r35533

Ping. This patch proposal has received no comments and I haven't  
noticed it being committed, either.

Gavin.


On 22/04/2009, at 8:47 AM, Kyle McKay wrote:

> [[[
> Fix ssh zombie problem introduced with revision 35533
>
> * subversion/libsvn_ra_svn/client.c
>  (make_tunnel): Fully detach tunnel process to avoid having it
>                 receive signals while restoring the original
>                 apr_pool_note_subprocess to avoid creating zombies.
> ]]]
>
> Updated so that WIN32 behavior remains the same after applying this
> patch as it was after revision 35533.  The earlier version of the
> patch inadvertently reverted WIN32 behavior to the pre-r35533 state.
>
> Kyle
>
>> As detailed in this thread:
>>
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1445592
>>
>> regarding the change introduced into client.c by revision 35533:
>>
>> http://svn.collab.net/viewvc/svn?view=revision&revision=35533
>>
>> On Mar 27, 2009, at 09:51, Hyrum K. Wright wrote:
>>> The case which drove r35533 was a user who uses ssh connection
>>> pooling for svn connections.
>>
>> Change r35533 removed the call to to apr_pool_note_subprocess
>> meaning that Subversion never reaps any of its children.  This may
>> be okay for a short-lived process, but is not okay for a long-lived
>> process such as a GUI tool that has linked with the Subversion
>> library.
>>
>> If the GUI tool runs long enough, it can create so many un-reaped
>> zombie processes that system resources are exhausted and it becomes
>> impossible to spawn any new processes.
>>
>> The attached patch restores the original call to
>> apr_pool_note_subprocess thus guaranteeing no zombies are ever
>> created.  It also arranges for the tunnel process to become detached
>> so that it can "die in piece, in its own time, on its own terms" as
>> the log comment for revision 35533 mentions, but without causing
>> zombie processes to be created.
>>
>> This change retains full compatibility with ssh connection pooling
>> while eliminating the zombie problem.  See issue #2580 for the
>> original impetus for the change made in revision 35533.
>>
>> http://subversion.tigris.org/issues/show_bug.cgi?id=2580
>>
>> Kyle
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1849826 
> <client.c-patch_v2>

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

Re: [PATCH] (updated) fix for ssh zombies introduced with r35533

Posted by Gavin Baumanis <ga...@thespidernet.com>.
Hi Hyrum,

Thanks for the update.
I'll double check the issue and add the patch iif it's missing.

Gavin.

On 01/05/2009, at 10:41 PM, Hyrum K. Wright wrote:

> I'm currently testing (or rather, waiting for feedback from a couple  
> of people who are testing) this patch in a large deployment with ssh  
> connection sharing.  I plan to continue tracking this, but in case I  
> get hit by a bus, I believe the patch is attached to the issue, so  
> it won't get lost.
>
> -Hyrum
>
> On May 1, 2009, at 5:55 AM, Gavin Baumanis wrote:
>
>> Ping. This patch proposal has received no comments and I haven't
>> noticed it being committed, either.
>>
>> Gavin.
>>
>>
>> On 22/04/2009, at 8:47 AM, Kyle McKay wrote:
>>
>>> [[[
>>> Fix ssh zombie problem introduced with revision 35533
>>>
>>> * subversion/libsvn_ra_svn/client.c
>>> (make_tunnel): Fully detach tunnel process to avoid having it
>>>               receive signals while restoring the original
>>>               apr_pool_note_subprocess to avoid creating zombies.
>>> ]]]
>>>
>>> Updated so that WIN32 behavior remains the same after applying this
>>> patch as it was after revision 35533.  The earlier version of the
>>> patch inadvertently reverted WIN32 behavior to the pre-r35533 state.
>>>
>>> Kyle
>>>
>>>> As detailed in this thread:
>>>>
>>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1445592
>>>>
>>>> regarding the change introduced into client.c by revision 35533:
>>>>
>>>> http://svn.collab.net/viewvc/svn?view=revision&revision=35533
>>>>
>>>> On Mar 27, 2009, at 09:51, Hyrum K. Wright wrote:
>>>>> The case which drove r35533 was a user who uses ssh connection
>>>>> pooling for svn connections.
>>>>
>>>> Change r35533 removed the call to to apr_pool_note_subprocess
>>>> meaning that Subversion never reaps any of its children.  This may
>>>> be okay for a short-lived process, but is not okay for a long-lived
>>>> process such as a GUI tool that has linked with the Subversion
>>>> library.
>>>>
>>>> If the GUI tool runs long enough, it can create so many un-reaped
>>>> zombie processes that system resources are exhausted and it becomes
>>>> impossible to spawn any new processes.
>>>>
>>>> The attached patch restores the original call to
>>>> apr_pool_note_subprocess thus guaranteeing no zombies are ever
>>>> created.  It also arranges for the tunnel process to become  
>>>> detached
>>>> so that it can "die in piece, in its own time, on its own terms" as
>>>> the log comment for revision 35533 mentions, but without causing
>>>> zombie processes to be created.
>>>>
>>>> This change retains full compatibility with ssh connection pooling
>>>> while eliminating the zombie problem.  See issue #2580 for the
>>>> original impetus for the change made in revision 35533.
>>>>
>>>> http://subversion.tigris.org/issues/show_bug.cgi?id=2580
>>>>
>>>> Kyle
>>>
>>> ------------------------------------------------------
>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1849826
>>> <client.c-patch_v2>
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2012775
>

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

Re: [PATCH] (updated) fix for ssh zombies introduced with r35533

Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
I'm currently testing (or rather, waiting for feedback from a couple  
of people who are testing) this patch in a large deployment with ssh  
connection sharing.  I plan to continue tracking this, but in case I  
get hit by a bus, I believe the patch is attached to the issue, so it  
won't get lost.

-Hyrum

On May 1, 2009, at 5:55 AM, Gavin Baumanis wrote:

> Ping. This patch proposal has received no comments and I haven't
> noticed it being committed, either.
>
> Gavin.
>
>
> On 22/04/2009, at 8:47 AM, Kyle McKay wrote:
>
>> [[[
>> Fix ssh zombie problem introduced with revision 35533
>>
>> * subversion/libsvn_ra_svn/client.c
>> (make_tunnel): Fully detach tunnel process to avoid having it
>>                receive signals while restoring the original
>>                apr_pool_note_subprocess to avoid creating zombies.
>> ]]]
>>
>> Updated so that WIN32 behavior remains the same after applying this
>> patch as it was after revision 35533.  The earlier version of the
>> patch inadvertently reverted WIN32 behavior to the pre-r35533 state.
>>
>> Kyle
>>
>>> As detailed in this thread:
>>>
>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1445592
>>>
>>> regarding the change introduced into client.c by revision 35533:
>>>
>>> http://svn.collab.net/viewvc/svn?view=revision&revision=35533
>>>
>>> On Mar 27, 2009, at 09:51, Hyrum K. Wright wrote:
>>>> The case which drove r35533 was a user who uses ssh connection
>>>> pooling for svn connections.
>>>
>>> Change r35533 removed the call to to apr_pool_note_subprocess
>>> meaning that Subversion never reaps any of its children.  This may
>>> be okay for a short-lived process, but is not okay for a long-lived
>>> process such as a GUI tool that has linked with the Subversion
>>> library.
>>>
>>> If the GUI tool runs long enough, it can create so many un-reaped
>>> zombie processes that system resources are exhausted and it becomes
>>> impossible to spawn any new processes.
>>>
>>> The attached patch restores the original call to
>>> apr_pool_note_subprocess thus guaranteeing no zombies are ever
>>> created.  It also arranges for the tunnel process to become detached
>>> so that it can "die in piece, in its own time, on its own terms" as
>>> the log comment for revision 35533 mentions, but without causing
>>> zombie processes to be created.
>>>
>>> This change retains full compatibility with ssh connection pooling
>>> while eliminating the zombie problem.  See issue #2580 for the
>>> original impetus for the change made in revision 35533.
>>>
>>> http://subversion.tigris.org/issues/show_bug.cgi?id=2580
>>>
>>> Kyle
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1849826
>> <client.c-patch_v2>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2012775

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