TpProxy: rewrite signal listening

As with async calls, we now use GVariant, don't take ownership of
the arguments in the wrapper function, do more work in
hand-written code and less work in generated code, and don't
need to mess about with DBusGProxy or idles because GDBus is just
better.

I've moved monitoring the TpProxy's lifetime from "signal connection
listens for invalidated signal" to "TpProxy explicitly drops signal
connections" in order to avoid depending on dispose emitting
invalidated, which was always a design flaw really. That also gives
us the opportunity to move the invocation of "drop all signal
connections" into an idle, which is necessary so that other listeners
to the same signal that invalidated the object (e.g. StatusChanged)
will still be notified.

7 files changed, 227 insertions, 466 deletions

diff --git a/docs/reference/telepathy-glib/telepathy-glib-sections.txt b/docs/reference/telepathy-glib/telepathy-glib-sections.txt
index 26fa64b4f..5f842b8dc 100644
--- a/docs/reference/telepathy-glib/telepathy-glib-sections.txt
+++ b/docs/reference/telepathy-glib/telepathy-glib-sections.txt
@@ -3009,10 +3009,8 @@ tp_proxy_invalidate
 TpProxyInterfaceAddedCb
 <SUBSECTION>
 TpProxyWrapperFunc
-TpProxyInvokeFunc
 tp_proxy_pending_call_v1_new
-tp_proxy_signal_connection_v0_new
-tp_proxy_signal_connection_v0_take_results
+tp_proxy_signal_connection_v1_new
 </SECTION>
 
 <SECTION>
diff --git a/telepathy-glib/core-proxy.c b/telepathy-glib/core-proxy.c
index bc8a57795..ef70dcc86 100644
--- a/telepathy-glib/core-proxy.c
+++ b/telepathy-glib/core-proxy.c
@@ -60,12 +60,11 @@ tp_proxy_pending_call_v1_new (TpProxy *proxy,
 }
 
 TpProxySignalConnection *
-tp_proxy_signal_connection_v0_new (TpProxy *self,
+tp_proxy_signal_connection_v1_new (TpProxy *self,
     GQuark iface,
     const gchar *member,
-    const GType *expected_types,
-    GCallback collect_args,
-    TpProxyInvokeFunc invoke_callback,
+    const GVariantType *expected_types,
+    TpProxyWrapperFunc wrapper,
     GCallback callback,
     gpointer user_data,
     GDestroyNotify destroy,
@@ -74,16 +73,8 @@ tp_proxy_signal_connection_v0_new (TpProxy *self,
 {
   g_assert (_tp_proxy_implementation.version != NULL);
   return _tp_proxy_implementation.signal_connection_new (self, iface, member,
-      expected_types, collect_args, invoke_callback, callback, user_data,
-      destroy, weak_object, error);
-}
-
-void
-tp_proxy_signal_connection_v0_take_results (TpProxySignalConnection *sc,
-    GValueArray *args)
-{
-  g_assert (_tp_proxy_implementation.version != NULL);
-  _tp_proxy_implementation.signal_connection_take_results (sc, args);
+      expected_types, wrapper, callback, user_data, destroy, weak_object,
+      error);
 }
 
 void
@@ -98,7 +89,6 @@ tp_private_proxy_set_implementation (TpProxyImplementation *impl)
   g_assert (impl->check_interface_by_id != NULL);
   g_assert (impl->pending_call_new != NULL);
   g_assert (impl->signal_connection_new != NULL);
-  g_assert (impl->signal_connection_take_results != NULL);
 
   memcpy (&_tp_proxy_implementation, impl, sizeof (TpProxyImplementation));
 
diff --git a/telepathy-glib/proxy-internal.h b/telepathy-glib/proxy-internal.h
index 8cd48a574..ec06e7f2e 100644
--- a/telepathy-glib/proxy-internal.h
+++ b/telepathy-glib/proxy-internal.h
@@ -47,16 +47,13 @@ typedef struct {
     TpProxySignalConnection *(*signal_connection_new) (TpProxy *,
         GQuark,
         const gchar *,
-        const GType *,
-        GCallback,
-        TpProxyInvokeFunc,
+        const GVariantType *,
+        TpProxyWrapperFunc,
         GCallback,
         gpointer,
         GDestroyNotify,
         GObject *,
         GError **);
-    void (*signal_connection_take_results) (TpProxySignalConnection *,
-        GValueArray *);
 
     GType type;
 } TpProxyImplementation;
@@ -78,21 +75,18 @@ _tp_proxy_pending_call_v1_new (TpProxy *proxy,
     GDestroyNotify destroy,
     GObject *weak_object);
 
-TpProxySignalConnection *_tp_proxy_signal_connection_new (TpProxy *self,
+TpProxySignalConnection *
+_tp_proxy_signal_connection_v1_new (TpProxy *self,
     GQuark iface,
     const gchar *member,
-    const GType *expected_types,
-    GCallback collect_args,
-    TpProxyInvokeFunc invoke_callback,
+    const GVariantType *expected_types,
+    TpProxyWrapperFunc wrapper,
     GCallback callback,
     gpointer user_data,
     GDestroyNotify destroy,
     GObject *weak_object,
     GError **error);
 
-void _tp_proxy_signal_connection_take_results (TpProxySignalConnection *sc,
-    GValueArray *args);
-
 /*
  * Implemented in the -core library, and called by the -main library.
  *
@@ -123,4 +117,9 @@ gboolean _tp_proxy_will_announce_connected_finish (TpProxy *self,
 void _tp_proxy_ensure_factory (gpointer self,
     TpClientFactory *factory);
 
+void _tp_proxy_add_signal_connection (TpProxy *self,
+    TpProxySignalConnection *sc);
+void _tp_proxy_remove_signal_connection (TpProxy *self,
+    TpProxySignalConnection *sc);
+
 #endif
diff --git a/telepathy-glib/proxy-signals.c b/telepathy-glib/proxy-signals.c
index 2e389ae55..fd5531382 100644
--- a/telepathy-glib/proxy-signals.c
+++ b/telepathy-glib/proxy-signals.c
@@ -40,80 +40,76 @@
  * Since: 0.7.1
  */
 
-typedef struct _TpProxySignalInvocation TpProxySignalInvocation;
-
-struct _TpProxySignalInvocation {
-    TpProxySignalConnection *sc;
-    TpProxy *proxy;
-    GValueArray *args;
-    guint idle_source;
-};
-
 struct _TpProxySignalConnection {
     /* 1 if D-Bus has us
-     * 1 per member of @invocations
-     * 1 per callback being invoked right now */
+     * 1 if per callback being invoked (possibly nested!) right now */
     gsize refcount;
 
-    /* borrowed ref (discarded when we see invalidated signal)
-     * + 1 per member of @invocations
-     * + 1 per callback being invoked (possibly nested!) right now */
+    /* (transfer full) */
+    GDBusConnection *conn;
+
     TpProxy *proxy;
 
-    DBusGProxy *iface_proxy;
-    gchar *member;
+    guint id;
+    GVariantType *expected_types;
     GCallback collect_args;
-    TpProxyInvokeFunc invoke_callback;
+    TpProxyWrapperFunc wrapper;
     GCallback callback;
     gpointer user_data;
     GDestroyNotify destroy;
     GObject *weak_object;
-    /* queue of _TpProxySignalInvocation, not including any that are
-     * being invoked right now */
-    GQueue invocations;
 };
 
-static void _tp_proxy_signal_connection_dgproxy_destroy (DBusGProxy *,
-    TpProxySignalConnection *);
+static void tp_proxy_signal_connection_unref (TpProxySignalConnection *sc);
 
-static void
-tp_proxy_signal_connection_disconnect_dbus_glib (TpProxySignalConnection *sc)
+/**
+ * tp_proxy_signal_connection_disconnect:
+ * @sc: a signal connection
+ *
+ * Disconnect the given signal connection. After this function returns, you
+ * must not assume that the signal connection remains valid, but you must not
+ * explicitly free it either.
+ *
+ * It is not safe to call this function if @sc has been disconnected already,
+ * which happens in each of these situations:
+ *
+ * <itemizedlist>
+ * <listitem>the @weak_object used when @sc was created has been
+ *  destroyed</listitem>
+ * <listitem>tp_proxy_signal_connection_disconnect has already been
+ *  used</listitem>
+ * <listitem>the proxy has been invalidated</listitem>
+ * </itemizedlist>
+ *
+ * Since: 0.7.1
+ */
+void
+tp_proxy_signal_connection_disconnect (TpProxySignalConnection *sc)
 {
-  DBusGProxy *iface_proxy = sc->iface_proxy;
+  guint id;
 
   /* ignore if already done */
-  if (iface_proxy == NULL)
-    return;
-
-  sc->iface_proxy = NULL;
-  g_signal_handlers_disconnect_by_func (iface_proxy,
-      _tp_proxy_signal_connection_dgproxy_destroy, sc);
-  dbus_g_proxy_disconnect_signal (iface_proxy, sc->member,
-      sc->collect_args, (gpointer) sc);
-
-  g_object_unref (iface_proxy);
-}
+  if (sc->id == 0)
+    {
+      DEBUG ("%p: already done, ignoring", sc);
+      return;
+    }
 
-static void
-tp_proxy_signal_connection_proxy_invalidated (TpProxy *proxy,
-                                              guint domain,
-                                              gint code,
-                                              const gchar *message,
-                                              TpProxySignalConnection *sc)
-{
-  g_assert (sc != NULL);
-  g_assert (domain != 0);
-  g_assert (message != NULL);
+  DEBUG ("%p", sc);
 
-  DEBUG ("%p: TpProxy %p invalidated (I have %p): %s", sc, proxy,
-      sc->proxy, message);
-  g_assert (proxy == sc->proxy);
+  id = sc->id;
+  sc->id = 0;
 
-  g_signal_handlers_disconnect_by_func (sc->proxy,
-      tp_proxy_signal_connection_proxy_invalidated, sc);
-  sc->proxy = NULL;
+  if (sc->proxy != NULL)
+    {
+      _tp_proxy_remove_signal_connection (sc->proxy, sc);
+      sc->proxy = NULL;
+    }
 
-  tp_proxy_signal_connection_disconnect_dbus_glib (sc);
+  /* likely to free @sc */
+  sc->refcount++;
+  g_dbus_connection_signal_unsubscribe (sc->conn, id);
+  tp_proxy_signal_connection_unref (sc);
 }
 
 static void
@@ -128,6 +124,9 @@ tp_proxy_signal_connection_lost_weak_ref (gpointer data,
 
   sc->weak_object = NULL;
 
+  /* don't wrap this in a ref/unref, because we might already have had the
+   * last-unref as a result of our TpProxy disappearing, in which case
+   * we are waiting for an idle to avoid fd.o #14750 */
   tp_proxy_signal_connection_disconnect (sc);
 }
 
@@ -136,6 +135,8 @@ _tp_proxy_signal_connection_finish_free (gpointer p)
 {
   TpProxySignalConnection *sc = p;
 
+  DEBUG ("%p", sc);
+
   if (sc->weak_object != NULL)
     {
       g_object_weak_unref (sc->weak_object,
@@ -148,34 +149,30 @@ _tp_proxy_signal_connection_finish_free (gpointer p)
   return FALSE;
 }
 
-/* Return TRUE if it dies. */
-static gboolean
+static void
 tp_proxy_signal_connection_unref (TpProxySignalConnection *sc)
 {
   if (--(sc->refcount) > 0)
     {
       MORE_DEBUG ("%p: %" G_GSIZE_FORMAT " refs left", sc, sc->refcount);
-      return FALSE;
+      return;
     }
 
   MORE_DEBUG ("removed last ref to %p", sc);
 
   if (sc->proxy != NULL)
     {
-      g_signal_handlers_disconnect_by_func (sc->proxy,
-          tp_proxy_signal_connection_proxy_invalidated, sc);
+      _tp_proxy_remove_signal_connection (sc->proxy, sc);
       sc->proxy = NULL;
     }
 
-  g_assert (sc->invocations.length == 0);
-
   if (sc->destroy != NULL)
     sc->destroy (sc->user_data);
 
   sc->destroy = NULL;
   sc->user_data = NULL;
-
-  g_free (sc->member);
+  g_clear_object (&sc->conn);
+  g_clear_pointer (&sc->expected_types, g_variant_type_free);
 
   /* We can't inline this here, because of fd.o #14750. If our signal
    * connection gets destroyed by side-effects of something else losing a
@@ -186,157 +183,60 @@ tp_proxy_signal_connection_unref (TpProxySignalConnection *sc)
    * signal connection until we've re-entered the main loop. */
   g_idle_add_full (G_PRIORITY_HIGH, _tp_proxy_signal_connection_finish_free,
       sc, NULL);
-
-  return TRUE;
-}
-
-/**
- * tp_proxy_signal_connection_disconnect:
- * @sc: a signal connection
- *
- * Disconnect the given signal connection. After this function returns, you
- * must not assume that the signal connection remains valid, but you must not
- * explicitly free it either.
- *
- * It is not safe to call this function if @sc has been disconnected already,
- * which happens in each of these situations:
- *
- * <itemizedlist>
- * <listitem>the @weak_object used when @sc was created has been
- *  destroyed</listitem>
- * <listitem>tp_proxy_signal_connection_disconnect has already been
- *  used</listitem>
- * <listitem>the proxy has been invalidated</listitem>
- * </itemizedlist>
- *
- * Since: 0.7.1
- */
-void
-tp_proxy_signal_connection_disconnect (TpProxySignalConnection *sc)
-{
-  TpProxySignalInvocation *invocation;
-
-  while ((invocation = g_queue_pop_head (&sc->invocations)) != NULL)
-    {
-      g_assert (invocation->sc == sc);
-      g_object_unref (invocation->proxy);
-      invocation->proxy = NULL;
-      invocation->sc = NULL;
-      g_source_remove (invocation->idle_source);
-
-      if (tp_proxy_signal_connection_unref (sc))
-        return;
-    }
-
-  tp_proxy_signal_connection_disconnect_dbus_glib (sc);
 }
 
 static void
-tp_proxy_signal_invocation_free (gpointer p)
+tp_proxy_signal_connection_cb (GDBusConnection *connection,
+    const gchar *sender_name,
+    const gchar *object_path,
+    const gchar *interface_name,
+    const gchar *signal_name,
+    GVariant *parameters,
+    gpointer user_data)
 {
-  TpProxySignalInvocation *invocation = p;
+  TpProxySignalConnection *sc = user_data;
+  TpProxy *proxy;
+
+  DEBUG ("%p: %s.%s from %s:%s", sc, interface_name, signal_name, sender_name,
+      object_path);
 
-  if (invocation->sc != NULL)
+  if (!g_variant_is_of_type (parameters, sc->expected_types))
     {
-      /* this shouldn't really happen - it'll get run if the idle source
-       * is removed by something other than t_p_s_c_disconnect or
-       * t_p_s_i_run */
-      WARNING ("idle source removed by someone else");
-
-      g_queue_remove (&invocation->sc->invocations, invocation);
-      g_object_unref (invocation->proxy);
-      tp_proxy_signal_connection_unref (invocation->sc);
+      DEBUG ("... expected parameters of type '%.*s', got '%s', ignoring",
+          (int) g_variant_type_get_string_length (sc->expected_types),
+          g_variant_type_peek_string (sc->expected_types),
+          g_variant_get_type_string (parameters));
+      return;
     }
 
-  g_assert (invocation->proxy == NULL);
-
-  if (invocation->args != NULL)
-    tp_value_array_free (invocation->args);
-
-  g_slice_free (TpProxySignalInvocation, invocation);
-}
-
-static gboolean
-tp_proxy_signal_invocation_run (gpointer p)
-{
-  TpProxySignalInvocation *invocation = p;
-  TpProxySignalInvocation *popped = g_queue_pop_head
-      (&invocation->sc->invocations);
-
-  /* if GLib is running idle handlers in the wrong order, then we've lost */
-  MORE_DEBUG ("%p: popped %p", invocation->sc, popped);
-  g_assert (popped == invocation);
-
-  invocation->sc->invoke_callback (invocation->proxy, NULL,
-      invocation->args, invocation->sc->callback, invocation->sc->user_data,
-      invocation->sc->weak_object);
-
-  /* the invoke callback steals args */
-  invocation->args = NULL;
-
-  /* there's one ref to the proxy per queued invocation, to keep it
-   * alive */
-  MORE_DEBUG ("%p refcount-- due to %p run, sc=%p", invocation->proxy,
-      invocation, invocation->sc);
-  g_object_unref (invocation->proxy);
-  invocation->proxy = NULL;
-  tp_proxy_signal_connection_unref (invocation->sc);
-  invocation->sc = NULL;
-
-  return FALSE;
-}
-
-static void
-tp_proxy_signal_connection_dropped (gpointer p,
-                                    GClosure *unused)
-{
-  TpProxySignalConnection *sc = p;
-
-  MORE_DEBUG ("%p (%u invocations queued)", sc, sc->invocations.length);
+  /* we shouldn't get here if we already disconnected the GDBus-level
+   * signal connection... */
+  g_assert (sc->id != 0);
+  /* ... or if the proxy has already been disposed, which disconnects
+   * the GDBus-level signal connection */
+  g_assert (sc->proxy != NULL);
 
+  /* The callback might invalidate proxy, which disconnects the signal
+   * and sets sc->proxy to NULL, so we need to use a temporary here */
+  sc->refcount++;
+  proxy = g_object_ref (sc->proxy);
+  sc->wrapper (proxy, NULL, parameters,
+      sc->callback, sc->user_data, sc->weak_object);
+  g_object_unref (proxy);
   tp_proxy_signal_connection_unref (sc);
 }
 
-static void
-_tp_proxy_signal_connection_dgproxy_destroy (DBusGProxy *iface_proxy,
-                                             TpProxySignalConnection *sc)
-{
-  g_assert (iface_proxy != NULL);
-  g_assert (sc != NULL);
-  g_assert (sc->iface_proxy == iface_proxy);
-
-  DEBUG ("%p: DBusGProxy %p invalidated", sc, iface_proxy);
-
-  sc->iface_proxy = NULL;
-  g_signal_handlers_disconnect_by_func (iface_proxy,
-      _tp_proxy_signal_connection_dgproxy_destroy, sc);
-  g_object_unref (iface_proxy);
-}
-
-static void
-collect_none (DBusGProxy *dgproxy, TpProxySignalConnection *sc)
-{
-  _tp_proxy_signal_connection_take_results (sc, NULL);
-}
-
 /**
- * tp_proxy_signal_connection_v0_new:
+ * tp_proxy_signal_connection_v1_new:
  * @self: a proxy
  * @iface: a quark whose string value is the D-Bus interface
  * @member: the name of the signal to which we're connecting
  * @expected_types: an array of expected GTypes for the arguments, terminated
  *  by %G_TYPE_INVALID
- * @collect_args: a callback to be given to dbus_g_proxy_connect_signal(),
- *  which must marshal the arguments into a #GValueArray and use them to call
- *  tp_proxy_signal_connection_v0_take_results(); this callback is not
- *  guaranteed to be called by future versions of telepathy-glib, which might
- *  be able to implement its functionality internally. If no arguments are
- *  expected at all (expected_types = { G_TYPE_INVALID }) then this callback
- *  should instead be %NULL
- * @invoke_callback: a function which will be called with @error = %NULL,
+ * @wrapper: a function which will be called with @error = %NULL,
  *  which should invoke @callback with @user_data, @weak_object and other
  *  appropriate arguments taken from @args
- * @callback: user callback to be invoked by @invoke_callback
+ * @callback: user callback to be invoked by @wrapper
  * @user_data: user-supplied data for the callback
  * @destroy: user-supplied destructor for the data, which will be called
  *   when the signal connection is disconnected for any reason,
@@ -347,7 +247,7 @@ collect_none (DBusGProxy *dgproxy, TpProxySignalConnection *sc)
  * @error: If not %NULL, used to raise an error if %NULL is returned
  *
  * Allocate a new structure representing a signal connection, and connect to
- * the signal, arranging for @invoke_callback to be called when it arrives.
+ * the signal, arranging for @wrapper to be called when it arrives.
  *
  * This function is for use by #TpProxy subclass implementations only, and
  * should usually only be called from code generated by
@@ -362,12 +262,11 @@ collect_none (DBusGProxy *dgproxy, TpProxySignalConnection *sc)
 /* that's implemented in the core library, but it calls this: */
 
 TpProxySignalConnection *
-_tp_proxy_signal_connection_new (TpProxy *self,
+_tp_proxy_signal_connection_v1_new (TpProxy *self,
     GQuark iface,
     const gchar *member,
-    const GType *expected_types,
-    GCallback collect_args,
-    TpProxyInvokeFunc invoke_callback,
+    const GVariantType *expected_types,
+    TpProxyWrapperFunc wrapper,
     GCallback callback,
     gpointer user_data,
     GDestroyNotify destroy,
@@ -375,10 +274,8 @@ _tp_proxy_signal_connection_new (TpProxy *self,
     GError **error)
 {
   TpProxySignalConnection *sc;
-  DBusGProxy *iface_proxy = tp_proxy_get_interface_by_id (self,
-      iface, error);
 
-  if (iface_proxy == NULL)
+  if (!tp_proxy_check_interface_by_id (self, iface, error))
     {
       if (destroy != NULL)
         destroy (user_data);
@@ -386,89 +283,36 @@ _tp_proxy_signal_connection_new (TpProxy *self,
       return NULL;
     }
 
-  if (expected_types[0] == G_TYPE_INVALID)
-    {
-      collect_args = G_CALLBACK (collect_none);
-    }
-  else
-    {
-      g_return_val_if_fail (collect_args != NULL, NULL);
-    }
-
   sc = g_slice_new0 (TpProxySignalConnection);
 
-  MORE_DEBUG ("(proxy=%p, if=%s, sig=%s, collect=%p, invoke=%p, "
-      "cb=%p, ud=%p, dn=%p, wo=%p) -> %p",
-      self, g_quark_to_string (iface), member, collect_args,
-      invoke_callback, callback, user_data, destroy, weak_object, sc);
-
   sc->refcount = 1;
+  sc->expected_types = g_variant_type_copy (expected_types);
+  sc->conn = g_object_ref (tp_proxy_get_dbus_connection (self));
   sc->proxy = self;
-  sc->iface_proxy = g_object_ref (iface_proxy);
-  sc->member = g_strdup (member);
-  sc->collect_args = collect_args;
-  sc->invoke_callback = invoke_callback;
+  sc->wrapper = wrapper;
   sc->callback = callback;
   sc->user_data = user_data;
   sc->destroy = destroy;
   sc->weak_object = weak_object;
 
+  DEBUG ("%p: %s.%s from %s:%s %p",
+      sc, g_quark_to_string (iface), member, tp_proxy_get_bus_name (self),
+      tp_proxy_get_object_path (self), self);
+
   if (weak_object != NULL)
     g_object_weak_ref (weak_object, tp_proxy_signal_connection_lost_weak_ref,
         sc);
 
-  g_signal_connect (self, "invalidated",
-      G_CALLBACK (tp_proxy_signal_connection_proxy_invalidated), sc);
-
-  g_signal_connect (iface_proxy, "destroy",
-      G_CALLBACK (_tp_proxy_signal_connection_dgproxy_destroy), sc);
-
-  dbus_g_proxy_connect_signal (iface_proxy, member, collect_args, sc,
-      tp_proxy_signal_connection_dropped);
-
+  sc->id = g_dbus_connection_signal_subscribe (sc->conn,
+      tp_proxy_get_bus_name (self),
+      g_quark_to_string (iface),
+      member,
+      tp_proxy_get_object_path (self),
+      NULL, /* arg0 */
+      G_DBUS_SIGNAL_FLAGS_NONE,
+      tp_proxy_signal_connection_cb,
+      sc,
+      (GDestroyNotify) tp_proxy_signal_connection_unref);
+  _tp_proxy_add_signal_connection (self, sc);
   return sc;
 }
-
-/**
- * tp_proxy_signal_connection_v0_take_results:
- * @sc: The signal connection
- * @args: The arguments of the signal
- *
- * Feed the results of a signal invocation back into the signal connection
- * machinery.
- *
- * This method should only be called from #TpProxy subclass implementations,
- * in the callback that implements @collect_args.
- *
- * Since: 0.7.1
- */
-
-/* that's implemented in the core library, but it calls this: */
-
-void
-_tp_proxy_signal_connection_take_results (TpProxySignalConnection *sc,
-    GValueArray *args)
-{
-  TpProxySignalInvocation *invocation = g_slice_new0 (TpProxySignalInvocation);
-  /* FIXME: assert that the GValueArray is the right length, or
-   * even that it contains the right types? */
-
-  /* as long as there are queued invocations, we keep one ref to the TpProxy
-   * and one ref to the TpProxySignalConnection per invocation */
-  MORE_DEBUG ("%p refcount++ due to %p, sc=%p", sc->proxy, invocation, sc);
-  invocation->proxy = g_object_ref (sc->proxy);
-  sc->refcount++;
-
-  invocation->sc = sc;
-  invocation->args = args;
-
-  g_queue_push_tail (&sc->invocations, invocation);
-
-  MORE_DEBUG ("invocations: head=%p tail=%p count=%u",
-      sc->invocations.head, sc->invocations.tail,
-      sc->invocations.length);
-
-  invocation->idle_source = g_idle_add_full (G_PRIORITY_HIGH,
-      tp_proxy_signal_invocation_run, invocation,
-      tp_proxy_signal_invocation_free);
-}
diff --git a/telepathy-glib/proxy-subclass.h b/telepathy-glib/proxy-subclass.h
index c485f01e0..f143a1d54 100644
--- a/telepathy-glib/proxy-subclass.h
+++ b/telepathy-glib/proxy-subclass.h
@@ -32,9 +32,6 @@ G_BEGIN_DECLS
 typedef void (*TpProxyWrapperFunc) (TpProxy *self,
     const GError *error, GVariant *args,
     GCallback callback, gpointer user_data, GObject *weak_object);
-typedef void (*TpProxyInvokeFunc) (TpProxy *self,
-    GError *error, GValueArray *args, GCallback callback, gpointer user_data,
-    GObject *weak_object);
 
 TpProxyPendingCall *tp_proxy_pending_call_v1_new (TpProxy *self,
     gint timeout_ms, GQuark iface, const gchar *member,
@@ -42,16 +39,12 @@ TpProxyPendingCall *tp_proxy_pending_call_v1_new (TpProxy *self,
     GCallback callback, gpointer user_data, GDestroyNotify destroy,
     GObject *weak_object);
 
-TpProxySignalConnection *tp_proxy_signal_connection_v0_new (TpProxy *self,
-    GQuark iface, const gchar *member,
-    const GType *expected_types,
-    GCallback collect_args, TpProxyInvokeFunc invoke_callback,
+TpProxySignalConnection *tp_proxy_signal_connection_v1_new (TpProxy *self,
+    GQuark iface, const gchar *member, const GVariantType *expected_types,
+    TpProxyWrapperFunc wrapper,
     GCallback callback, gpointer user_data, GDestroyNotify destroy,
     GObject *weak_object, GError **error);
 
-void tp_proxy_signal_connection_v0_take_results
-    (TpProxySignalConnection *sc, GValueArray *args);
-
 GDBusProxy *tp_proxy_get_interface_by_id (TpProxy *self, GQuark iface,
     GError **error);
 
diff --git a/telepathy-glib/proxy.c b/telepathy-glib/proxy.c
index b4aa420f4..62e959b72 100644
--- a/telepathy-glib/proxy.c
+++ b/telepathy-glib/proxy.c
@@ -355,6 +355,9 @@ struct _TpProxyPrivate {
 
     TpClientFactory *factory;
 
+    /* used as a set, may be %NULL */
+    GHashTable *signal_connections;
+
     gulong gdbus_closed_signal;
     guint unique_name_watch;
 };
@@ -537,6 +540,36 @@ tp_proxy_has_interface (gpointer self,
 
 static void tp_proxy_poll_features (TpProxy *self, const GError *error);
 
+/* Signature chosen to be a GSourceFunc */
+static gboolean
+tp_proxy_disconnect_all_signals (gpointer p)
+{
+  TpProxy *self = p;
+  GHashTableIter iter;
+  GHashTable *signal_connections;
+  gpointer sc;
+
+  DEBUG ("%p", self);
+
+  signal_connections = self->priv->signal_connections;
+  /* make _tp_proxy_remove_signal_connection do nothing */
+  self->priv->signal_connections = NULL;
+
+  if (signal_connections == NULL)
+    return FALSE;
+
+  g_hash_table_iter_init (&iter, signal_connections);
+
+  while (g_hash_table_iter_next (&iter, &sc, NULL))
+    {
+      g_hash_table_iter_remove (&iter);
+      tp_proxy_signal_connection_disconnect (sc);
+    }
+
+  g_hash_table_unref (signal_connections);
+  return FALSE;
+}
+
 /**
  * tp_proxy_invalidate:
  * @self: a proxy
@@ -592,6 +625,21 @@ tp_proxy_invalidate (TpProxy *self, const GError *error)
    * to the proxies */
   g_datalist_clear (&self->priv->interfaces);
   g_clear_object (&self->priv->dbus_connection);
+
+  /* Don't disconnect D-Bus signal handlers until we go back to the main
+   * loop, so that if this is called in response to StatusChanged or similar,
+   * any other handlers to StatusChanged get a chance to run.
+   *
+   * This is deliberately the same priority with which GDBus schedules
+   * signal callbacks, so it happens immediately after the signal callbacks
+   * themselves. */
+  if (self->priv->signal_connections != NULL)
+    {
+      DEBUG ("%p: disconnecting signal handlers later", self);
+      g_idle_add_full (G_PRIORITY_DEFAULT,
+          tp_proxy_disconnect_all_signals, g_object_ref (self),
+          g_object_unref);
+    }
 }
 
 static void
@@ -1042,6 +1090,10 @@ tp_proxy_dispose (GObject *object)
 
   DEBUG ("%p", self);
 
+  /* Do this explicitly here, so that we're at least not relying on
+   * tp_proxy_invalidate() for this one thing. */
+  tp_proxy_disconnect_all_signals (self);
+
   /* One day we should stop doing this. When that day comes, we need
    * to make sure the cleanup from tp_proxy_invalidate() is duplicated
    * here, and is idempotent. */
@@ -2143,3 +2195,25 @@ _tp_proxy_will_announce_connected_finish (TpProxy *self,
 {
   _tp_implement_finish_void (self, _tp_proxy_will_announce_connected_async)
 }
+
+void
+_tp_proxy_add_signal_connection (TpProxy *self,
+    TpProxySignalConnection *sc)
+{
+  g_assert (self->priv->invalidated == NULL);
+
+  if (self->priv->signal_connections == NULL)
+    {
+      self->priv->signal_connections = g_hash_table_new (NULL, NULL);
+    }
+
+  g_hash_table_add (self->priv->signal_connections, sc);
+}
+
+void
+_tp_proxy_remove_signal_connection (TpProxy *self,
+    TpProxySignalConnection *sc)
+{
+  if (self->priv->signal_connections != NULL)
+    g_hash_table_remove (self->priv->signal_connections, sc);
+}
diff --git a/tools/glib-client-gen.py b/tools/glib-client-gen.py
index 8eba83c31..0b7978423 100644
--- a/tools/glib-client-gen.py
+++ b/tools/glib-client-gen.py
@@ -113,7 +113,7 @@ class Generator(object):
 
         arg_count = 0
         args = []
-        out_args = []
+        arg_sig = []
 
         for arg in signal.getElementsByTagName('arg'):
             name = arg.getAttribute('name')
@@ -128,11 +128,10 @@ class Generator(object):
 
             info = type_to_gtype(type)
             args.append((name, info, tp_type, arg))
+            arg_sig.append(type)
 
         callback_name = ('%s_%s_signal_callback_%s'
                          % (self.prefix_lc, iface_lc, member_lc))
-        collect_name = ('_%s_%s_collect_args_of_%s'
-                        % (self.prefix_lc, iface_lc, member_lc))
         invoke_name = ('_%s_%s_invoke_callback_for_%s'
                        % (self.prefix_lc, iface_lc, member_lc))
 
@@ -182,56 +181,24 @@ class Generator(object):
 
         self.h('    gpointer user_data, GObject *weak_object);')
 
-        if args:
-            self.b('static void')
-            self.b('%s (DBusGProxy *proxy G_GNUC_UNUSED,' % collect_name)
-
-            for arg in args:
-                name, info, tp_type, elt = arg
-                ctype, gtype, marshaller, pointer = info
-
-                const = pointer and 'const ' or ''
-
-                self.b('    %s%s%s,' % (const, ctype, name))
-
-            self.b('    TpProxySignalConnection *sc)')
-            self.b('{')
-            self.b('  G_GNUC_BEGIN_IGNORE_DEPRECATIONS')
-            self.b('  GValueArray *args = g_value_array_new (%d);' % len(args))
-            self.b('  GValue blank = { 0 };')
-            self.b('  guint i;')
-            self.b('')
-            self.b('  g_value_init (&blank, G_TYPE_INT);')
-            self.b('')
-            self.b('  for (i = 0; i < %d; i++)' % len(args))
-            self.b('    g_value_array_append (args, &blank);')
-            self.b('  G_GNUC_END_IGNORE_DEPRECATIONS')
-            self.b('')
-
-            for i, arg in enumerate(args):
-                name, info, tp_type, elt = arg
-                ctype, gtype, marshaller, pointer = info
-
-                self.b('  g_value_unset (args->values + %d);' % i)
-                self.b('  g_value_init (args->values + %d, %s);' % (i, gtype))
-
-                self.b('  ' + copy_into_gvalue('args->values + %d' % i,
-                    gtype, marshaller, name))
-                self.b('')
-
-            self.b('  tp_proxy_signal_connection_v0_take_results (sc, args);')
-            self.b('}')
-
         self.b('static void')
         self.b('%s (TpProxy *tpproxy,' % invoke_name)
-        self.b('    GError *error G_GNUC_UNUSED,')
-        self.b('    GValueArray *args,')
+        self.b('    const GError *error G_GNUC_UNUSED,')
+        self.b('    GVariant *variant,')
         self.b('    GCallback generic_callback,')
         self.b('    gpointer user_data,')
         self.b('    GObject *weak_object)')
         self.b('{')
         self.b('  %s callback =' % callback_name)
         self.b('      (%s) generic_callback;' % callback_name)
+
+        if args:
+            self.b('  GValue args_val = G_VALUE_INIT;')
+            self.b('  GValueArray *args_va;')
+            self.b('')
+            self.b('  dbus_g_value_parse_g_variant (variant, &args_val);')
+            self.b('  args_va = g_value_get_boxed (&args_val);')
+
         self.b('')
         self.b('  if (callback != NULL)')
         self.b('    callback (g_object_ref (tpproxy),')
@@ -241,20 +208,14 @@ class Generator(object):
             ctype, gtype, marshaller, pointer = info
 
             getter = value_getter(gtype, marshaller)
-            self.b('      %s (args->values + %d),' % (getter, i))
+            self.b('      %s (args_va->values + %d),' % (getter, i))
 
         self.b('      user_data,')
         self.b('      weak_object);')
         self.b('')
 
-        self.b('  G_GNUC_BEGIN_IGNORE_DEPRECATIONS')
-        if len(args) > 0:
-            self.b('  g_value_array_free (args);')
-        else:
-            self.b('  if (args != NULL)')
-            self.b('    g_value_array_free (args);')
-            self.b('')
-        self.b('  G_GNUC_END_IGNORE_DEPRECATIONS')
+        if args:
+            self.b('  g_value_unset (&args_val);')
 
         self.b('  g_object_unref (tpproxy);')
         self.b('}')
@@ -320,27 +281,11 @@ class Generator(object):
         self.b('    GObject *weak_object,')
         self.b('    GError **error)')
         self.b('{')
-        self.b('  GType expected_types[%d] = {' % (len(args) + 1))
-
-        for arg in args:
-            name, info, tp_type, elt = arg
-            ctype, gtype, marshaller, pointer = info
-
-            self.b('      %s,' % gtype)
-
-        self.b('      G_TYPE_INVALID };')
-        self.b('')
         self.b('  g_return_val_if_fail (callback != NULL, NULL);')
         self.b('')
-        self.b('  return tp_proxy_signal_connection_v0_new ((TpProxy *) proxy,')
+        self.b('  return tp_proxy_signal_connection_v1_new ((TpProxy *) proxy,')
         self.b('      %s, \"%s\",' % (self.get_iface_quark(), member))
-        self.b('      expected_types,')
-
-        if args:
-            self.b('      G_CALLBACK (%s),' % collect_name)
-        else:
-            self.b('      NULL, /* no args => no collector function */')
-
+        self.b('      G_VARIANT_TYPE ("(%s)"),' % ''.join(arg_sig))
         self.b('      %s,' % invoke_name)
         self.b('      G_CALLBACK (callback), user_data, destroy,')
         self.b('      weak_object, error);')
@@ -981,23 +926,6 @@ class Generator(object):
         b('}')
         b('')
 
-    def do_signal_add(self, signal):
-        marshaller_items = []
-        gtypes = []
-
-        for i in signal.getElementsByTagName('arg'):
-            name = i.getAttribute('name')
-            type = i.getAttribute('type')
-            info = type_to_gtype(type)
-            # type, GType, STRING, is a pointer
-            gtypes.append(info[1])
-
-        self.b('  dbus_g_proxy_add_signal (proxy, "%s",'
-               % signal.getAttribute('name'))
-        for gtype in gtypes:
-            self.b('      %s,' % gtype)
-        self.b('      G_TYPE_INVALID);')
-
     def do_interface(self, node):
         ifaces = node.getElementsByTagName('interface')
         assert len(ifaces) == 1
@@ -1013,24 +941,6 @@ class Generator(object):
         signals = node.getElementsByTagName('signal')
         methods = node.getElementsByTagName('method')
 
-        if signals:
-            self.b('static inline void')
-            self.b('%s_add_signals_for_%s (DBusGProxy *proxy)'
-                    % (self.prefix_lc, name.lower()))
-            self.b('{')
-
-            if self.tp_proxy_api >= (0, 7, 6):
-                self.b('  if (!tp_proxy_dbus_g_proxy_claim_for_signal_adding '
-                       '(proxy))')
-                self.b('    return;')
-
-            for signal in signals:
-                self.do_signal_add(signal)
-
-            self.b('}')
-            self.b('')
-            self.b('')
-
         for signal in signals:
             self.do_signal(name, signal)
 
@@ -1064,53 +974,6 @@ class Generator(object):
         for node in nodes:
             self.do_interface(node)
 
-        if self.group is not None:
-            self.h('void %s_%s_add_signals (TpProxy *self,'
-                    % (self.prefix_lc, self.group))
-            self.h('    guint quark,')
-            self.h('    DBusGProxy *proxy,')
-            self.h('    gpointer unused);')
-            self.h('')
-
-            self.b('/*')
-            self.b(' * %s_%s_add_signals:' % (self.prefix_lc, self.group))
-            self.b(' * @self: the #TpProxy')
-            self.b(' * @quark: a quark whose string value is the interface')
-            self.b(' *   name whose signals should be added')
-            self.b(' * @proxy: the D-Bus proxy to which to add the signals')
-            self.b(' * @unused: not used for anything')
-            self.b(' *')
-            self.b(' * Tell dbus-glib that @proxy has the signatures of all')
-            self.b(' * signals on the given interface, if it\'s one we')
-            self.b(' * support.')
-            self.b(' *')
-            self.b(' * This function should be used as a signal handler for')
-            self.b(' * #TpProxy::interface-added.')
-            self.b(' */')
-            self.b('void')
-            self.b('%s_%s_add_signals (TpProxy *self G_GNUC_UNUSED,'
-                    % (self.prefix_lc, self.group))
-            self.b('    guint quark,')
-            self.b('    DBusGProxy *proxy,')
-            self.b('    gpointer unused G_GNUC_UNUSED)')
-
-            self.b('{')
-
-            for node in nodes:
-                iface = node.getElementsByTagName('interface')[0]
-                self.iface_dbus = iface.getAttribute('name')
-                signals = node.getElementsByTagName('signal')
-                if not signals:
-                    continue
-                name = node.getAttribute('name').replace('/', '').lower()
-                self.iface_uc = name.upper()
-                self.b('  if (quark == %s)' % self.get_iface_quark())
-                self.b('    %s_add_signals_for_%s (proxy);'
-                       % (self.prefix_lc, name))
-
-            self.b('}')
-            self.b('')
-
         self.h('G_END_DECLS')
         self.h('')