Login | Register
My pages Projects Community openCollabNet

Discussions > dev [DISABLED] > Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn

subversion
Discussion topic

Back to topic list

Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn

Author mackyle
Full name Kyle McKay
Date 2009-05-07 06:56:56 PDT
Message On May 7, 2009, at 00:20, Bert Huijben <rhuijben at sharpsvn dot net> wrote:
>> -----Original Message-----
>> From: Hyrum K. Wright [mailto:hyrum at hyrumwright dot org]
>> Sent: woensdag 6 mei 2009 18:31
>> To: svn at subversion dot tigris dot org
>> Subject: svn commit: r37615 - trunk/subversion/libsvn_ra_svn
>>
>> Author: hwright
>> Date: Wed May 6 09:31:28 2009
>> New Revision: 37615
>>
>> Log:
>> A better fix to issue #2580.
>>
>> Fix ssh zombie problem introduced with revision 35533
>>
>> * subversion/libsvn_ra​_svn/client.c
>> (detach_child_cleanup): New.
>> (make_tunnel): Fully detach tunnel process to avoid having it
>> receive
>> signals
>> while restoring the original apr_pool_note_subprocess to avoid
>> creating
>> zombies.
>>
>> Patch by: Kyle McKay <mackyle at gmail dot com>
>>
>> Modified:
>> trunk/subversion/lib​svn_ra_svn/client.c
>>
>> Modified: trunk/subversion/lib​svn_ra_svn/client.c
>> URL:
>> http://svn.collab.ne​t/viewvc/svn/trunk/s​ubversion/libsvn_ra_​svn/client
>> .
>> c?pathrev=37615&​r1=37614&r2=3761​5
>> =
>> =
>> ====================​====================​====================​=========
>> =======
>> --- trunk/subversion/lib​svn_ra_svn/client.c Wed May 6 09:02:16 2009
>> (r37614)
>> +++ trunk/subversion/lib​svn_ra_svn/client.c Wed May 6 09:31:28 2009
>> (r37615)
>> @@ -26,6 +26,12 @@
>> #include <apr_strings.h>
>> #include <apr_network_io.h>
>> #include <apr_uri.h>
>> +#if APR_HAVE_STDLIB_H
>> +#include <stdlib.h>
>> +#endif
>> +#if APR_HAVE_UNISTD_H
>> +#include <unistd.h>
>> +#endif
>>
>> #include "svn_types.h"
>> #include "svn_string.h"
>> @@ -426,6 +432,43 @@ static void handle_child_process_error(a
>> svn_error_clear(svn_​ra_svn_flush(conn, pool));
>> }
>>
>> +static apr_status_t detach_child_cleanup(void *data)
>> +{
>> +#if APR_HAVE_STDLIB_H
>> +#if APR_HAVE_UNISTD_H
>> +#if APR_HAS_FORK
>
> I don't think we need the two outer #if's. (We only need the check for
> fork()).

The exit function is declared in stdlib.h. It's not appropriate to
call it if we haven't included the header where it's defined (hence
the test for APR_HAVE_STDLIB_H).

The fork function is declared in unistd.h, it's not appropriate to try
and call it if we haven't included the header where it's defined
(hence the test for APR_HAVE_UNISTD_H).

For this patch it's also not appropriate to call fork if APR isn't
using it (hence the test for APR_HAS_FORK).

So the patch code is only compiled in if you have an stdlib.h (for
exit()) and a unistd.h (for fork()) AND APR is using fork.

Are you certain that there aren't some APR-supported and Subversion
supported platforms where APR_HAS_FORK is true but one or the other of
the standard headers that declare fork/exit are missing? Some older
systems perhaps...

> These defines check whether a specific header file exist and not if
> functionality exist.

Actually the tests are there to avoid calling functions for which we
haven't included the header file which declares them.

> Using these checks is most likely a dumb guess on whether this is
> running on
> a Unix system. (stdlib.h is available on any operating system I know
> and
> probably part of recent C standards).
>
> Bert

If stdlib.h is available everywhere, then why does apr define a test
for it rather than just always use it?

Since apr does define a test for APR_HAVE_STDLIB_H, that suggests apr
supports systems without it and that the patch code should too.

Kyle
Attachments

« Previous message in topic | 1 of 5 | Next message in topic »

Messages

Show all messages in topic

Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn mackyle Kyle McKay 2009-05-07 06:56:56 PDT
     RE: svn commit: r37615 - trunk/subversion/libsvn_ra_svn rhuijben Bert Huijben 2009-05-07 07:53:28 PDT
         Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn mackyle Kyle McKay 2009-05-07 08:37:28 PDT
         Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn hwright Hyrum K. Wright 2009-05-07 08:38:25 PDT
             Re: svn commit: r37615 - trunk/subversion/libsvn_ra_svn mackyle Kyle McKay 2009-05-07 08:49:25 PDT
Messages per page: