edbus: Fix bug found in refactored banshee example

Refactor edbus_signal_handler_add() so internal signal handlers don't
set the connection free callback. This fixes the bug in which
EDBus_Connection was freeing the signal handler of EDBus_Conenction_Name

==22814== Invalid read of size 4
==22814==    at 0x40564B0: edbus_signal_handler_del (edbus_signal_handler.c:278)
==22814==    by 0x4040E65: _edbus_connection_name_unref (edbus_core.c:507)
==22814==    by 0x404106B: edbus_connection_name_owner_monitor (edbus_core.c:520)
==22814==    by 0x4055F63: _edbus_signal_handler_clean (edbus_signal_handler.c:217)
==22814==    by 0x40564F8: edbus_signal_handler_del (edbus_signal_handler.c:279)
==22814==    by 0x4043088: _edbus_connection_unref (edbus_core.c:1045)
==22814==    by 0x404352F: edbus_connection_unref (edbus_core.c:1105)
==22814==    by 0x80498AA: main (banshee.c:233)
==22814==  Address 0x44bea48 is 0 bytes inside a block of size 72 free'd
==22814==    at 0x402C06C: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==22814==    by 0x4056118: _edbus_signal_handler_del (edbus_signal_handler.c:249)
==22814==    by 0x4056401: edbus_signal_handler_unref (edbus_signal_handler.c:272)
==22814==    by 0x4056503: edbus_signal_handler_del (edbus_signal_handler.c:280)
==22814==    by 0x4043088: _edbus_connection_unref (edbus_core.c:1045)
==22814==    by 0x404352F: edbus_connection_unref (edbus_core.c:1105)
==22814==    by 0x80498AA: main (banshee.c:233)
==22814==
CRI<22814>: src/lib/edbus_signal_handler.c:278 edbus_signal_handler_del() *** Eina Magic Check Failed !!!
    Input handle has already been freed!
    *** NAUGHTY PROGRAMMER!!!
    *** SPANK SPANK SPANK!!!
    *** Now go fix your code. Tut tut tut!

Patch by: José Roberto de Souza  <zehortigoza@profusion.mobi>

SVN revision: 80686

diff --git a/src/lib/edbus_core.c b/src/lib/edbus_core.c
index f46f050..c18fdeb 100644 (file)
--- a/src/lib/edbus_core.c
+++ b/src/lib/edbus_core.c
@@ -499,11 +499,11 @@ edbus_connection_name_get(EDBus_Connection *conn, const char *name)
         goto end;
      }
    edbus_name_owner_get(conn, cn->name, on_get_name_owner, cn);
-   cn->name_owner_changed = edbus_signal_handler_add(conn, EDBUS_FDO_BUS,
-                                                     EDBUS_FDO_PATH,
-                                                     EDBUS_FDO_INTERFACE,
-                                                     "NameOwnerChanged",
-                                                     on_name_owner_changed, cn);
+   cn->name_owner_changed = _edbus_signal_handler_add(conn, EDBUS_FDO_BUS,
+                                                      EDBUS_FDO_PATH,
+                                                      EDBUS_FDO_INTERFACE,
+                                                      "NameOwnerChanged",
+                                                      on_name_owner_changed, cn);
    edbus_signal_handler_match_extra_set(cn->name_owner_changed, "arg0",
                                         cn->name, NULL);
 
@@ -1000,13 +1000,6 @@ _edbus_connection_unref(EDBus_Connection *conn)
    EINA_INLIST_FOREACH_SAFE(conn->pendings, list, p)
      edbus_pending_cancel(p);
 
-   while (conn->signal_handlers)
-     {
-        list = conn->signal_handlers;
-        h = EINA_INLIST_CONTAINER_GET(conn->signal_handlers, EDBus_Signal_Handler);
-        edbus_signal_handler_del(h);
-     }
-
    iter = eina_hash_iterator_data_new(conn->names);
    EINA_ITERATOR_FOREACH(iter, cn)
      {
@@ -1033,14 +1026,21 @@ _edbus_connection_unref(EDBus_Connection *conn)
         CRITICAL("Connection %p released with live pending calls!",
                  conn);
         EINA_INLIST_FOREACH(conn->pendings, p)
-          ERR("conn=%p alive pending call=%p dest=%s path=%s %s.%s()",
-              conn, p,
+          ERR("conn=%p alive pending call=%p dest=%s path=%s %s.%s()", conn, p,
               edbus_pending_destination_get(p),
               edbus_pending_path_get(p),
               edbus_pending_interface_get(p),
               edbus_pending_method_get(p));
      }
 
+   if (conn->signal_handlers)
+     {
+        CRITICAL("Connection %p released with live signal handlers!", conn);
+        EINA_INLIST_FOREACH(conn->signal_handlers, h)
+          ERR("conn=%p alive signal=%p %s.%s path=%s", conn, h, h->interface,
+              h->member, h->path);
+     }
+
    for (i = 0; i < EDBUS_CONNECTION_EVENT_LAST; i++)
      {
         EDBus_Connection_Context_Event *ce = conn->event_handlers + i;

diff --git a/src/lib/edbus_object.c b/src/lib/edbus_object.c
index 9f6c704..3ccc138 100644 (file)
--- a/src/lib/edbus_object.c
+++ b/src/lib/edbus_object.c
@@ -399,10 +399,10 @@ edbus_object_event_callback_add(EDBus_Object *obj, EDBus_Object_Event_Type type,
             if (obj->interfaces_added)
               break;
             obj->interfaces_added =
-                     edbus_signal_handler_add(obj->conn, obj->name, NULL,
-                                              EDBUS_FDO_INTERFACE_OBJECT_MANAGER,
-                                              "InterfacesAdded",
-                                              _cb_interfaces_added, obj);
+                     _edbus_signal_handler_add(obj->conn, obj->name, NULL,
+                                               EDBUS_FDO_INTERFACE_OBJECT_MANAGER,
+                                               "InterfacesAdded",
+                                               _cb_interfaces_added, obj);
             EINA_SAFETY_ON_NULL_RETURN(obj->interfaces_added);
             edbus_signal_handler_match_extra_set(obj->interfaces_added, "arg0",
                                                  obj->path, NULL);
@@ -413,10 +413,10 @@ edbus_object_event_callback_add(EDBus_Object *obj, EDBus_Object_Event_Type type,
            if (obj->interfaces_removed)
              break;
            obj->interfaces_removed =
-                    edbus_signal_handler_add(obj->conn, obj->name, NULL,
-                                             EDBUS_FDO_INTERFACE_OBJECT_MANAGER,
-                                             "InterfacesRemoved",
-                                             _cb_interfaces_removed, obj);
+                    _edbus_signal_handler_add(obj->conn, obj->name, NULL,
+                                              EDBUS_FDO_INTERFACE_OBJECT_MANAGER,
+                                              "InterfacesRemoved",
+                                              _cb_interfaces_removed, obj);
            EINA_SAFETY_ON_NULL_RETURN(obj->interfaces_added);
            edbus_signal_handler_match_extra_set(obj->interfaces_removed,
                                                 "arg0", obj->path, NULL);
@@ -598,8 +598,8 @@ edbus_object_signal_handler_add(EDBus_Object *obj, const char *interface, const
    EDBUS_OBJECT_CHECK_RETVAL(obj, NULL);
    EINA_SAFETY_ON_NULL_RETURN_VAL(cb, NULL);
 
-   handler = edbus_signal_handler_add(obj->conn, obj->name, obj->path,
-                                      interface, member, cb, cb_data);
+   handler = _edbus_signal_handler_add(obj->conn, obj->name, obj->path,
+                                       interface, member, cb, cb_data);
    EINA_SAFETY_ON_NULL_RETURN_VAL(handler, NULL);
 
    edbus_signal_handler_cb_free_add(handler, _on_signal_handler_free, obj);

diff --git a/src/lib/edbus_private.h b/src/lib/edbus_private.h
index 41adc17..d87cc95 100644 (file)
--- a/src/lib/edbus_private.h
+++ b/src/lib/edbus_private.h
@@ -77,6 +77,8 @@ Eina_Value            *_message_iter_struct_to_eina_value(EDBus_Message_Iter *it
 
 void                   edbus_connection_name_ref(EDBus_Connection_Name *cn);
 void                   edbus_connection_name_unref(EDBus_Connection *conn, EDBus_Connection_Name *cn);
+EDBus_Signal_Handler  *_edbus_signal_handler_add(EDBus_Connection *conn, const char *sender, const char *path, const char *interface, const char *member, EDBus_Signal_Cb cb, const void *cb_data);
+
 
 #ifdef HAVE_VA_LIST_AS_ARRAY
 #define MAKE_PTR_FROM_VA_LIST(arg) ((va_list *)(arg))

diff --git a/src/lib/edbus_proxy.c b/src/lib/edbus_proxy.c
index 25305e4..3b6525f 100644 (file)
--- a/src/lib/edbus_proxy.c
+++ b/src/lib/edbus_proxy.c
@@ -605,8 +605,8 @@ edbus_proxy_signal_handler_add(EDBus_Proxy *proxy, const char *member, EDBus_Sig
    name = edbus_object_bus_name_get(proxy->obj);
    path = edbus_object_path_get(proxy->obj);
 
-   handler = edbus_signal_handler_add(proxy->obj->conn, name, path,
-                                      proxy->interface, member, cb, cb_data);
+   handler = _edbus_signal_handler_add(proxy->obj->conn, name, path,
+                                       proxy->interface, member, cb, cb_data);
    EINA_SAFETY_ON_NULL_RETURN_VAL(handler, NULL);
 
    edbus_signal_handler_cb_free_add(handler, _on_signal_handler_free, proxy);

diff --git a/src/lib/edbus_signal_handler.c b/src/lib/edbus_signal_handler.c
index af3600d..7c2b081 100644 (file)
--- a/src/lib/edbus_signal_handler.c
+++ b/src/lib/edbus_signal_handler.c
@@ -148,10 +148,38 @@ edbus_signal_handler_match_extra_set(EDBus_Signal_Handler *sh, ...)
    return ret;
 }
 
+static void _on_handler_of_conn_free(void *data, const void *dead_pointer);
+
+static void
+_on_connection_free(void *data, const void *dead_pointer)
+{
+   EDBus_Signal_Handler *sh = data;
+   edbus_signal_handler_cb_free_del(sh, _on_handler_of_conn_free, sh->conn);
+   edbus_signal_handler_del(sh);
+}
+
+static void
+_on_handler_of_conn_free(void *data, const void *dead_pointer)
+{
+   EDBus_Connection *conn = data;
+   edbus_connection_cb_free_del(conn, _on_connection_free, dead_pointer);
+}
+
 EAPI EDBus_Signal_Handler *
 edbus_signal_handler_add(EDBus_Connection *conn, const char *sender, const char *path, const char *interface, const char *member, EDBus_Signal_Cb cb, const void *cb_data)
 {
    EDBus_Signal_Handler *sh;
+   sh = _edbus_signal_handler_add(conn, sender, path, interface, member, cb, cb_data);
+   EINA_SAFETY_ON_NULL_RETURN_VAL(sh, NULL);
+   edbus_connection_cb_free_add(conn, _on_connection_free, sh);
+   edbus_signal_handler_cb_free_add(sh, _on_handler_of_conn_free, conn);
+   return sh;
+}
+
+EDBus_Signal_Handler *
+_edbus_signal_handler_add(EDBus_Connection *conn, const char *sender, const char *path, const char *interface, const char *member, EDBus_Signal_Cb cb, const void *cb_data)
+{
+   EDBus_Signal_Handler *sh;
    Eina_Strbuf *match;
    DBusError err;