On Sun, May 24, 2015 at 2:51 AM, James McCoy <jamessan_at_debian.org> wrote:
> The patch is working for me as well.
>
I've dug a little deeper and think I've found a serious flaw in how the
Perl bindings handle
the Perl arguments and return values stack.
Swig generates for Perl what is known (in the Perl world) as XS code:
basically C source with a heavy dose of macros and functions from the Perl
internals.
At issue here is the Perl arguments and return values stack, it's an array
where the (Perl) arguments for the generated function are found and
the (Perl) return values will be stored. The stack pointer is a complex
expression, so for performance reasons it's value is stored into a local
variable of the function (that's what the "dSP" does that Swig puts at the
start
of every generated function). All other macros that deal with the stack,
e.g. ST, XPUSHs, EXTEND, use this "local stack pointer".
This works under the assumption that the "global stack pointer" doesn't
change until the function returns. This assumption is violated when the
XS code "calls back" into Perl (using one of the Perl internal functions
call_method, call_sv or call_pv). During the call back, the Perl argument
stack might get expanded (i.e. reallocated), so local and global stack
pointers
get out of sync. You must be synchronize them right before and after
calling any call_* (using macros PUTBACK and SPAGAIN).
See the perlcall man page <http://perldoc.perl.org/perlcall.html> for the
gory details.
*No* Swig rules for the Perl bindings generate calls to any call_* function,
so no problem here. There is one universal wrapper for call_sv and
call_method:
function svn_swig_pl_callback_thunk in the Perl bindings helper library,
though.
It's itself XS code and AFAICT employs the correct magic around the actual
call of call_method or call_sv. This function *is* called in Swig rules.
But it does call back into Perl hence it *must* be treated the same way as
the
Perl native call_* functions, i.e. bracketed with PUTBACK/SPAGAIN.
Actually there is only one direct call to svn_swig_pl_callback_thunk in the
Swig rules. But it is used in a lot of other helper functions in
libsvn_swig_perl.
And hence these functions (and their callers, caller's callers etc)
*must* be bracketed, too, when used in Swig rules.
Failing to do so might work as long as the call back chain and the combined
stack usage stays small enough that the Perl runtime doesn't have to
reallocate its argument stack somewhere in between. IMHO James McCoy's
problem exceeded this threshold. My previous patch didn't really solve
the problem, it only reduced the callback chain (by getting rid of the
first return value from _wrap_svn_txdelta_apply which is a Perl object,
generated by a callback to Perl "SVN::MD5->new").
Cheers, Roderich
Received on 2015-05-27 19:57:16 CEST