You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kyle McKay <ma...@gmail.com> on 2009/04/21 22:47:33 UTC

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

[[[
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

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

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

Posted by Gavin Baumanis <ga...@thespidernet.com>.
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