Re: gperl_callback_invoke userdata copy

[Kevin Ryde <user42 zip com au>]

Torsten Schoenfeld <kaffeetisch gmx de> writes:
>
> I don't know if it will work -- did you try it?

I did now!

> Do we want to support assigning to $_[-1]?

It could be in the docs if it's meant to work.  I can't see any
disadvantage, it gets you a free bit of "state data" if you want to use
it that way.

I see GClosure for signals is this way -- refcnt inc and mortalize --
and assigning alters the stored data.  I guess GPerlCallback can be the
same.  The net change then to it is just a refcount fix which was
tickled by a callback disconnecting/destroying itself.

(Some test cases under in GtkLinkButton because there's no destroyable
GPerlCallbacks in plain Glib.)

--- GClosure.xs	23 May 2008 09:48:57 +1000	1.31
+++ GClosure.xs	03 Sep 2008 08:57:45 +1000	
@@ -402,26 +402,26 @@
 		}
 	}
 
-        /* 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.
+        /* Usual REFCNT_inc and 2mortal here for putting something on the
+         * stack.  It's possible callback->func will disconnect itself, in
+         * which case gperl_callback_destroy() will REFCNT_dec the data.
+         * That's fine, it leaves the mortal ref on the stack as the only
+         * one remaining, and the next FREETMPS will decrement and destroy
+         * in the usual way.
          *
-         * 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.
+         * Being a plain push here means callback->func can modify its
+         * $_[-1] to modify the stored userdata.  Slightly scary, but it's a
+         * cute way to get a free bit of per-connection data you can play
+         * with as a state variable or whatnot.  And not making a copy saves
+         * a couple of bytes of memory :-).
          */
-	if (callback->data)
-		XPUSHs (sv_mortalcopy (callback->data));
-
+	{
+		SV *data = callback->data;
+		if (data) {
+			SvREFCNT_inc_simple_void_NN (data);
+			XPUSHs (sv_2mortal (data));
+		}
+	}
 	va_end (var_args);
 
 	PUTBACK;

--- GtkLinkButton.t	02 Sep 2008 09:21:24 +1000	1.4
+++ GtkLinkButton.t	03 Sep 2008 09:28:23 +1000	
@@ -52,6 +52,48 @@
 
 $button->set_uri_hook (undef);
 
+{ my $saw_data;
+  sub hook2 {
+    $saw_data = $_[-1];
+    $_[-1]++;
+  }
+  $button->set_uri_hook (\&hook2, 100);
+  $button->clicked;
+  is ($saw_data, 100, 'initial userdata');
+  $button->clicked;
+  is ($saw_data, 101, 'incremented once');
+  $button->clicked;
+  is ($saw_data, 102, 'incremented twice');
+
+  $button->set_uri_hook (undef);
+}
+
+{ my $userdata = [ 'something' ];
+  sub hook3 {
+    $_[-1] = undef;
+  }
+  $button->set_uri_hook (\&hook3, $userdata);
+  require Scalar::Util;
+  Scalar::Util::weaken ($userdata);
+  is_deeply ($userdata, [ 'something' ],
+             'still alive when first weakened'); 
+  $button->clicked;
+  is ($userdata, undef, 'then gone when hook zaps its last arg'); 
+
+  $button->set_uri_hook (undef);
+}
+
+{ my $saw_data;
+  sub hook4 {
+    $button->set_uri_hook (undef);
+    $saw_data = $_[-1];
+  }
+  $button->set_uri_hook (\&hook4, [ 'hello' ]);
+  $button->clicked;
+  is_deeply ($saw_data, [ 'hello' ],
+             'userdata still ok when hook disconnects itself'); 
+}
+
 SKIP: {
 	skip 'new 2.14 stuff', 1
 		unless Gtk2->CHECK_VERSION(2, 13, 7); # FIXME: 2.14

-- 
The sigfile one-line movie reviews series:
"On the Beach" -- where better for the end of the world than Melbourne.