Bug 216034
Summary: | a bunch of my apps just crashed | ||||||
---|---|---|---|---|---|---|---|
Product: | Red Hat Enterprise Linux 5 | Reporter: | Ray Strode [halfline] <rstrode> | ||||
Component: | dbus-glib | Assignee: | David Zeuthen <davidz> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | |||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | 5.0 | CC: | jkubin, johnp, mclasen, otaylor, sgrubb, wtogami | ||||
Target Milestone: | --- | Keywords: | Desktop | ||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | RC | Doc Type: | Bug Fix | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2007-02-08 00:31:41 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 217766 | ||||||
Attachments: |
|
Description
Ray Strode [halfline]
2006-11-16 20:54:54 UTC
In fact, the backtrace is missing the dbus_g_proxy_manager_replace_name_owner frame it looks like. The crash is triggered by this block of code: │707 else │ │708 { │ │709 DBusGProxyNameOwnerInfo *info; │ │710 GSList *link; │ │711 │ │712 /* Name owner changed or deleted */ │ │713 │ │714 names = g_hash_table_lookup (manager->owner_names, prev_owner│ │715 │ >│716 link = g_slist_find_custom (names, name, find_name_in_info); │ │717 │ where name is "org.gnome.YelpService" and names looks bogus (gdb) p *(struct _GSList *) 0x8ed8f48 $18 = {data = 0x0, next = 0x0} Also, note this function (dbus_g_proxy_manager_replace_name_owner) has another code path that could conceivably add an element to the names list with null data: │716 link = g_slist_find_custom (names, name, find_name_in_info); │ │717 │ │718 info = NULL; │ │719 if (link != NULL) │ │720 { │ (fill in info here ...) │727 } | │728 │ │729 if (new_owner[0] == '\0') │ │730 { │ (...) │748 } │ │749 else │ │750 { │ │751 insert_nameinfo (manager, new_owner, info); │ │752 } and insert_nameinfo does │563 names = g_slist_append (names, info); │ I have no idea if it hit this code path earlier or not, I just noticed it when snooping around. *** Bug 215952 has been marked as a duplicate of this bug. *** This request was evaluated by Red Hat Product Management for inclusion in a Red Hat Enterprise Linux major release. Product Management has requested further review of this request by Red Hat Engineering, for potential inclusion in a Red Hat Enterprise Linux Major release. This request is not yet committed for inclusion. It also looks to me like info might be leaked in that function you cite: Here we remove info from the owner_names table: if (link != NULL) { info = link->data; names = g_slist_delete_link (names, link); if (names == NULL) g_hash_table_remove (manager->owner_names, prev_owner); } And here we do nothing with it, assuming new_owner is not empty: if (new_owner[0] == '\0') { DBusGProxyUnassociateData data; GSList *tmp; data.name = name; data.destroyed = NULL; /* A service went away, we need to unassociate proxies */ g_hash_table_foreach (manager->proxy_lists, unassociate_proxies, &data); UNLOCK_MANAGER (manager); for (tmp = data.destroyed; tmp; tmp = tmp->next) dbus_g_proxy_destroy (tmp->data); g_slist_free (data.destroyed); LOCK_MANAGER (manager); } I think info may need freeing in that case ? Created attachment 142259 [details]
possible patch
Here is an untested patch that adresses both issues.
Does that look reasonable, John ?
John says the patch looks good, and there very similar patches in upstream bugzilla. Thanks Matthias, I've included this patch * Tue Nov 28 2006 David Zeuthen <davidz> - 0.70-5 - Add dbus-glib-0.70-fix-info-leak.patch - Resolves: #216034 Package is partially built in Brew (waiting on s390x). Ray, can you verify that this package works? Thanks. I don't have a reliable way to reproduce the problem and it's only happened to me a few times, unfortunately. I wonder if we could write a test case that just takes control of a bus name and releases it over and over again. You don't want to take control and release it, instead you want to go from one owner to another owner. Calling : mugshot --replace a few times (it won't quit the old one with mugshot < 1.1.27, but it still replaces the D-Bus name) should take down your session pretty reliably. According to a conversation that Havoc and I had, the patch in the upstream bug report is most likely more correct than the one here ... the proxy code in D-Bus should simply not care changes in the name owner for names it doesn't have a proxy for, so inserting a dummy info entry doesn't make sense. (We didn't review it line by line.) A package has been built which should help the problem described in this bug report. This report is therefore being closed with a resolution of CURRENTRELEASE. You may reopen this bug report if the solution does not work for you. |