James McCoy <james...@debian.org> writes:
> On Wed, Mar 18, 2015 at 02:49:08PM +0000, Philip Martin wrote:
>> I also see that. Given that Perl has realloc'd some memory I suppose
>> it might be a reference counting bug in Subversion's XS code.
>>
>> Or perhaps the bug might be this code in svn_types.swg:
>>
>> %typemap(argout) unsigned char *result_digest {
>> /* FIXME: This code is clearly buggy. The return value of sv_newmortal()
>> is immediately overwritten by the return value
>> of svn_swig_pl_from_md5(). */
>> ST(argvi) = sv_newmortal();
>> ST(argvi++) = svn_swig_pl_from_md5($1);
>> }
>> #endif
>
> Indeed. Reading through some Perl documentation, the original commit,
> and svn_swig_pl_from_md5, I don't see why the svn_newmortal() call is
> there. svn_swig_pl_from_md5 already makes its return value mortal, so
> this is just allocating and overwriting a new SV*.
>
> Removing this resolves the crash and reduces the amount of
> (de)allocations.
Can you show us exactly what you changed. I tried this:
Index: ../src-1.9/subversion/bindings/swig/include/svn_types.swg
===================================================================
--- ../src-1.9/subversion/bindings/swig/include/svn_types.swg (revision
1668117)
+++ ../src-1.9/subversion/bindings/swig/include/svn_types.swg (working copy)
@@ -1119,7 +1119,6 @@
/* FIXME: This code is clearly buggy. The return value of sv_newmortal()
is immediately overwritten by the return value
of svn_swig_pl_from_md5(). */
- ST(argvi) = sv_newmortal();
ST(argvi++) = svn_swig_pl_from_md5($1);
}
#endif
and the crash still occurs in my 1.9 dev build.
I'm not familiar with this code, but looking at other code in the file I
tried this:
Index: ../src-1.9/subversion/bindings/swig/include/svn_types.swg
===================================================================
--- ../src-1.9/subversion/bindings/swig/include/svn_types.swg (revision
1668117)
+++ ../src-1.9/subversion/bindings/swig/include/svn_types.swg (working copy)
@@ -1119,8 +1119,7 @@
/* FIXME: This code is clearly buggy. The return value of sv_newmortal()
is immediately overwritten by the return value
of svn_swig_pl_from_md5(). */
- ST(argvi) = sv_newmortal();
- ST(argvi++) = svn_swig_pl_from_md5($1);
+ %append_output(svn_swig_pl_from_md5($1));
}
#endif
That does allow my dev build to run the clone for the first few
revisions, including the huge starting revision.
With that the generated code looks like:
ST(argvi) = sv_newmortal();
{
/* FIXME: This code is clearly buggy. The return value of sv_newmortal()
is immediately overwritten by the return value
of svn_swig_pl_from_md5(). */
if (argvi >= items) EXTEND(sp,1); ST(argvi) =
svn_swig_pl_from_md5(arg3); argvi++ ;
}
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
