Re: [PATCH] git-svn: Fix termination issues for remote svn connections

From: Kyle J. McKay <mackyle@gmail.com>
Date: Sep 6, 2013, 12:44 UTC
Message-ID: <E8831080-6FD3-4276-BE80-F0E9BAC65AA3@gmail.com>
In-Reply-To: <xmqqli3bhzop.fsf@gitster.dls.corp.google.com>

On Sep 5, 2013, at 11:48, Junio C Hamano wrote:

> I am Cc'ing Kyle McKay who apparently had some experience working
> with git-svn with newer svn that can only use serf, hoping that we
> can get an independent opinion/test just to be sure.

On Sep 3, 2013, at 00:35, Uli Heller wrote:

Show 17 quoted lines

> When using git-svn in combination with serf-1.2.1 core dumps are
> created on termination. This is caused by a bug in serf, a fix for
> the bug exists (see https://code.google.com/p/serf/source/detail?r=2146) 
> .
> Nevertheless, I think it makes sense to fix the issue within the
> git perl module Ra.pm, too. The change frees the private copy of
> the remote access object on termination which prevents the error
> from happening.
>
> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,
> the core dumps typically do show up when upgrading to a recent version
> of subversion.
>
> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.
> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and
> pointing me to that fix.
> ---

In addition to the needed 'Signed-off-by:' a mention should be made here of the need to use 'https:' to reproduce the problem.

Show 21 quoted lines

> perl/Git/SVN/Ra.pm | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index 75ecc42..78dd346 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -32,6 +32,11 @@ BEGIN {
> 	}
> }
>
> +END {
> +	$RA = undef;
> +	$ra_invalid = 1;
> +}
> +
> sub _auth_providers () {
> 	my @rv = (
> 	  SVN::Client::get_simple_provider(),
> -- 
> 1.8.4

Tested-by: Kyle J. McKay <mackyle@gmail.com>

I was able to reproduce the core dumps using subversion 1.8.3 (the most recent subversion release) with serf 1.3.1 (the most recent serf release) and verify that this patch eliminates the core dump at git- svn termination time.