| Summary: | mission-control-5 crashed with SIGABRT in raise() | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Pedro Villavicencio <pvillavi> |
| Component: | mission-control | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | critical | ||
| Priority: | medium | Keywords: | patch |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://cgit.collabora.com/git/user/wjt/telepathy-mission-control-wjt.git/log/?h=39767-client-unique_name-NULL | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | McdClient: don't allow unique_name to become NULL | ||
|
Description
Pedro Villavicencio
2011-08-02 08:32:17 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.
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. (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! Explanation in comment #3 seems correct to me, and the patch fixes that scenario. +1 Merged to 5.8 and master; thanks! 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.