| Summary: | [fixed in git] finalised DBusGConnection does not weak unref object | ||
|---|---|---|---|
| Product: | dbus | Reporter: | Christian Persch (GNOME) <chpe> |
| Component: | GLib | Assignee: | Rob Taylor <rob.taylor> |
| Status: | RESOLVED FIXED | QA Contact: | John (J5) Palmieri <johnp> |
| Severity: | critical | ||
| Priority: | high | CC: | walters |
| Version: | unspecified | Keywords: | patch |
| Hardware: | x86 (IA32) | ||
| OS: | Linux (All) | ||
| URL: | http://git.collabora.co.uk/?p=user/smcv/dbus-glib-smcv.git;a=shortlog;h=refs/heads/5688-survive-disconnection | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 21219 | ||
| Attachments: |
implementation
implementation regression test revised patch revised regression test Yet another iteration of the implementation Yet another iteration of the regression test |
||
|
Description
Christian Persch (GNOME)
2006-01-23 06:53:55 UTC
Joe can you take a look at this. Thanks. Why did I give this to joe? Reassigning. 3.2 years later, here's a patch for this "high priority" bug... I believe this patch to be correct, but I should give it some more testing with the telepathy-glib and telepathy-gabble regression tests before it's merged. I intend to use this code to implement dbus_g_connection_unregister_g_object(), for which I'll file a separate bug. http://git.collabora.co.uk/?p=user/smcv/dbus-glib-smcv.git;a=shortlog;h=refs/heads/5688-survive-disconnection git://git.collabora.co.uk/git/user/smcv/dbus-glib-smcv.git 5688-survive-disconnection Created attachment 24849 [details] [review] implementation Created attachment 24850 [details] [review] implementation Created attachment 24851 [details] [review] regression test Comment on attachment 24849 [details] [review] implementation Oops, attached the patch twice. Removing one. Created attachment 24852 [details] [review] revised patch The first patch I attached broke dbus_g_connection_lookup_g_object (), which apparently has no regression test coverage. Created attachment 24853 [details] [review] revised regression test Amended regression test which fixes the regression I caused in dbus_g_connection_lookup_g_object () Created attachment 24854 [details] [review] Yet another iteration of the implementation Fixes lookup of nonexistent objects. Created attachment 24855 [details] [review] Yet another iteration of the regression test Exercises lookup for nonexistent objects. The branch referenced above now fixes Bug #21219 too (take only the first two patches if that's undesirable). Haven't looked at the patch, but I am curious as to the situation in which this is happening. Is is a private DBusGConnection? Shutting down the session one to test memory leaks? Epiphany's crash does look like some strange corner case, perhaps involving a crashing session bus daemon (the current code is clearly wrong, though, even if in practice it only crashes in unusual circumstances). However, in the Telepathy project we would greatly benefit from being able to remove objects from the bus without relying on them being destroyed immediately (so an object's removal from the bus occurs immediately after it emits its "I'm dead" D-Bus signal, and can't get delayed by some irrelevant asynchronous operation that happens to hold a temporary ref to that object). This is Bug #21219. The situation causing this crash is more precisely "remove an object from the bus, *then* destroy it". As it happens, at the moment the only ways to remove an object from the bus are to destroy it (in which case the crash does not occur) or to disconnect from the bus (which usually makes your process call _exit() anyway - I don't like this but it's the API we have). However, as soon as #21219 is fixed, this assertion will occur every time the new object-removal API is used, hence fixing this lurking bug before addressing #21219 is quite important. This branch looks good to me, for what it's worth. Took a look at this patch. I was initially surprised we didn't just put the ObjectRegistration as "dbus_gobject_registration" data on the GObject, but instead still keep the path there and instead look up the registration from the connection. Anyways it's fine, if it's less code churn the way it is now that's good. Please commit. Fixed in git, thanks. Fixed in 0.82, according to Colin's release mail. |
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.