gperl_callback_invoke userdata copy

[Kevin Ryde <user42 zip com au>]

While nosing around for the tree filter modify callback I saw the
comment in gperl_callback_invoke about needing or not needing to copy
the userdata arg.  I came up with the degenerate program below; it
unhooks the GtkLinkButton callback from within a callback, and the
userdata arg goes away,

    userdata: ARRAY(0x83c131c)
    Use of uninitialized value in print at t-callback-data.pl line 15.
    userdata:

I wonder if it does, alas, have to do a copy to preserve the args for a
case like that.

use strict;
use warnings;
use Gtk2 '-init';

my $toplevel = Gtk2::Window->new('toplevel');
$toplevel->signal_connect (destroy => sub { Gtk2->main_quit });

my $button = Gtk2::LinkButton->new_with_label ('http:://foo.bar', 'Press me');
$toplevel->add ($button);

Gtk2::LinkButton->set_uri_hook (\&first_func, ['fsdjkljsdfkl']);
sub first_func {
  print "userdata: ",$_[2],"\n";
  Gtk2::LinkButton->set_uri_hook (undef, undef);
  print "userdata: ",$_[2],"\n";
}

$toplevel->show_all;
Gtk2->main;
exit 0;

--- GClosure.xs	12 May 2004 03:38:11 +1000	1.30
+++ GClosure.xs	03 May 2008 19:35:06 +1000	
@@ -401,14 +401,26 @@
 			XPUSHs (sv_2mortal (sv));
 		}
 	}
+
+        /* This is sv_mortalcopy() in case our GPerlCallback is destroyed
+         * from within the called func.  That can happen for instance with
+         * the uri handler on Gtk2::LinkButton.  If the called handler
+         * disconnects itself, or installs a new handler with different
+         * data, then gtk_link_button_set_uri_hook simply calls the destroy
+         * on the existing handler immediately, making
+         * gperl_callback_destroy do a SvREFCNT_dec on our callback->data.
+         * The symptom then is the pushed userdata arg $_[2] invalidated.
+         *
+         * If you're wondering why this doesn't arise with
+         * g_signal_handler_add_emission_hook funcs, it's because the signal
+         * emission mechanism has some trickery to defer the destroy of the
+         * callback until after it returns (it's held in a GHook and the ref
+         * count there is bumped for the duration of the call).  Presumably
+         * it's a matter of opinion whether deferring the destroy is good or
+         * bad, but sv_mortalcopy here copes with either.
+         */
 	if (callback->data)
-		/* my thinking on why i can just push this SV here...
-		 * if nobody keeps a reference to it (in the called function),
-		 * its refcount will be unaffected.  if they do take a ref,
-		 * that ref will be released at the end of that function.
-		 * so it just works out.  if any of this is untrue, change
-		 * this to XPUSHs (sv_2mortal (newSVsv (callback->data))); */
-		XPUSHs (callback->data);
+		XPUSHs (sv_mortalcopy (callback->data));
 
 	va_end (var_args);