Bug 39767 - mission-control-5 crashed with SIGABRT in raise()
Summary: mission-control-5 crashed with SIGABRT in raise()
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: Other All
: medium critical
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/wj...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-08-02 08:32 UTC by Pedro Villavicencio
Modified: 2011-10-04 07:55 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
McdClient: don't allow unique_name to become NULL (2.49 KB, patch)
2011-10-04 07:55 UTC, Will Thompson
Details | Splinter Review

Description Pedro Villavicencio 2011-08-02 08:32:17 UTC
this report has been filed here:

https://bugs.launchpad.net/ubuntu/+source/telepathy-mission-control-5/+bug/816808

.
Thread 1 (Thread 7858):
#0  0x00007fd181b06c55 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
        resultvar = 0
        pid = <value optimized out>
        selftid = 7858
#1  0x00007fd181b0aa06 in abort () at abort.c:92
        save_stage = 2
        act = {__sigaction_handler = {sa_handler = 0x7fd181e729a0 <_IO_2_1_stderr_>, sa_sigaction = 0x7fd181e729a0 <_IO_2_1_stderr_>}, sa_mask = {__val = {2048, 22436000, 140537804723258, 140537800530848, 140537826988032, 22549664, 22302896, 4294967295, 1, 0, 22549664, 3096640, 0, 140537807427152, 22067152, 140537804328960}}, sa_flags = -2094900060, sa_restorer = 0x1}
        sigs = {__val = {32, 0 <repeats 15 times>}}
#2  0x00007fd181ee127d in g_assertion_message (domain=<value optimized out>, file=<value optimized out>, line=<value optimized out>, func=0x453080 "_mcd_client_proxy_handler_get_all_cb", message=0x1549f60 "assertion failed: (self->priv->unique_name != NULL)") at /build/buildd/glib2.0-2.29.14/./glib/gtestutils.c:1425
        lstr = "667\000\377\177\000\000PBj\274\377\177\000\000\220\330Q\001\000\000\000\000\070)E\000\000\000\000"
        s = 0x15814a0 ""
#3  0x00007fd181ee17a2 in g_assertion_message_expr (domain=0x446801 "mcd", file=0x452408 "mcd-client.c", line=667, func=0x453080 "_mcd_client_proxy_handler_get_all_cb", expr=<value optimized out>) at /build/buildd/glib2.0-2.29.14/./glib/gtestutils.c:1436
        s = <value optimized out>
#4  0x000000000043bc16 in _mcd_client_proxy_handler_get_all_cb (proxy=<value optimized out>, properties=0x156bf00, error=0x0, p=<value optimized out>, o=<value optimized out>) at mcd-client.c:667
        self = 0x152d590
        bus_name = 0x1521990 "org.freedesktop.Telepathy.Client.GnomeShell._3a1_2e22.n0"
        filters = <value optimized out>
        handled_channels = <value optimized out>
        bypass = <value optimized out>
        __PRETTY_FUNCTION__ = "_mcd_client_proxy_handler_get_all_cb"
#5  0x00007fd182d66ad8 in _tp_cli_dbus_properties_invoke_callback_get_all (self=0x152d590, error=0x0, args=0x157d1a0, generic_callback=0x43b9e0 <_mcd_client_proxy_handler_get_all_cb>, user_data=<value optimized out>, weak_object=<value optimized out>) at _gen/tp-cli-generic-body.h:1190
        callback = 0x43b9e0 <_mcd_client_proxy_handler_get_all_cb>
#6  0x00007fd182d6d840 in tp_proxy_pending_call_idle_invoke (p=0x15436a0) at proxy-methods.c:153
        pc = 0x15436a0
        invoke = <value optimized out>
        __PRETTY_FUNCTION__ = "tp_proxy_pending_call_idle_invoke"
#7  0x00007fd181ebd5bd in g_main_dispatch (context=0x150b7d0) at /build/buildd/glib2.0-2.29.14/./glib/gmain.c:2500
        dispatch = 0x7fd181eb9290 <g_idle_dispatch>
        was_in_call = 0
        user_data = 0x15436a0
        callback = 0x7fd182d6d7d0 <tp_proxy_pending_call_idle_invoke>
        cb_funcs = 0x7fd18216d650
        cb_data = 0x153d6e0
        current_source_link = {data = 0x158d9c0, next = 0x0}
        need_destroy = <value optimized out>
        source = 0x158d9c0
        current = 0x1538c00
        i = <value optimized out>
#8  g_main_context_dispatch (context=0x150b7d0) at /build/buildd/glib2.0-2.29.14/./glib/gmain.c:3083
No locals.
#9  0x00007fd181ebddb8 in g_main_context_iterate (context=0x150b7d0, block=<value optimized out>, dispatch=1, self=<value optimized out>) at /build/buildd/glib2.0-2.29.14/./glib/gmain.c:3161
        max_priority = -100
        timeout = 0
        some_ready = 1
        nfds = 0
        allocated_nfds = <value optimized out>
        fds = <value optimized out>
#10 0x00007fd181ebe2f2 in g_main_loop_run (loop=0x1510110) at /build/buildd/glib2.0-2.29.14/./glib/gmain.c:3369
        __PRETTY_FUNCTION__ = "g_main_loop_run"
#11 0x000000000040e945 in main (argc=<value optimized out>, argv=<value optimized out>) at mc-server.c:78
        mcd = 0x1511010
Comment 1 Will Thompson 2011-10-03 09:05:02 UTC
The assertion (in _mcd_client_proxy_handler_get_all_cb) has this comment:

    /* by now, we at least know whether the client is running or not */
    g_assert (self->priv->unique_name != NULL);

Within McdClient itself, this assertion seems accurate: the first thing done, upon construction, is to add a name owner watch for the well-known name; the callback sets unique_name to "" if the name has no owner. The watch callback should definitely fire before the callback for the GetAll method calls made after the name owner watch is set up.

However… _mcd_client_proxy_set_active(), which is responsible for setting priv->unique_name, will set it to NULL if it is passed NULL. _mcd_client_registry_found_name() may call it with NULL, in the callback for a call to ListNames made at MC startup time…

I don't really understand the code in the client registry. It's not clear to me at all why the client registry should ever update the client's unique name—the client has its own name owner watch on the well-known name.
Comment 2 Will Thompson 2011-10-03 09:24:04 UTC
Here's a branch for review which should fix this crash. I'm unable to reproduce it myself, but I believe I've tracked down the only way the crash could occur.
Comment 3 Simon McVittie 2011-10-03 11:30:28 UTC
(In reply to comment #1)
> I don't really understand the code in the client registry. It's not clear to me
> at all why the client registry should ever update the client's unique name—the
> client has its own name owner watch on the well-known name.

If I remember correctly, the idea is that the client registry tells the client what's going on in two different situations:

* ListActivatableNames or ListNames told it that the client exists, but
  it doesn't know the unique name (and passes in NULL)
* NameOwnerChanged told it that the client exists *and* its unique name

I think the crashy case here might go something like this:

* ListActivatableNames reply says "Empathy is activatable"
* McdClientProxy created for Empathy
* GetNameOwner in the McdClientProxy wins the race with ListNames;
  McdClientProxy now knows its own unique name
* ListNames reply says "Empathy is on the bus"
* client registry tells the McdClientProxy that it is active, but
  not its unique name (because it doesn't know)
* non-NULL unique name from GetNameOwner is overwritten with NULL
* crash!
Comment 4 Xavier Claessens 2011-10-04 05:19:41 UTC
Explanation in comment #3 seems correct to me, and the patch fixes that scenario.

+1
Comment 5 Will Thompson 2011-10-04 07:54:45 UTC
Merged to 5.8 and master; thanks!
Comment 6 Will Thompson 2011-10-04 07:55:18 UTC
Created attachment 51962 [details] [review]
McdClient: don't allow unique_name to become NULL

This is the relevant patch, in case packagers want to apply it before releases happen.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.