Bug 22119 - [api-break] Proxies should not emit 'invalidated' when disposing.
Summary: [api-break] Proxies should not emit 'invalidated' when disposing.
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
Whiteboard: ABI-break
: 49375 (view as bug list)
Depends on:
Reported: 2009-06-06 04:40 UTC by Will Thompson
Modified: 2012-05-10 07:54 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Description Will Thompson 2009-06-06 04:40:03 UTC
I've just run into a bug in Empathy, whose root cause is that TpProxy emits 'invalidated' when it's being disposed. The bug is as follows:

* EmpathyTpContactList keeps a ref to each of a connection's Group channels in a hash table.
* When a Group channel is invalidated, it cleans up, and removes them from the hash table (unreffing them).

So far, so good. Now:

* If a new Group channel appears with the same name as an existing one (this could be argued to be the real bug in Empathy: I think this happened because the old Idle connection didn't die before Empathy tried to reconnect, so the old Group channel proxies were still hanging around), it replaces the old channel in the hash table.
* Replacing the old group unrefs it.
* The old group's refcount hits zero, so it's disposed, and emits Invalidated (raising its refcount temporarily to 1).
* Empathy's invalidated callback fires, and removes the group from the the hash table, unreffing it (a second time!).
* The refcount hits zero again, but TpChannel's dispose() is not idempotent so everyone dies (patch to follow).

I don't think that TpProxies should emit 'invalidated' when they're disposed. If you care about a proxy's validity, you should own a ref to it. Conflating an object's spiritual lifecycle (whether it's 'valid') with its C lifecycle seems to be the root of a number of bugs (see also: the conflation in CMs between channels being unreffed and emitting Closed, leading to hacks to break loops: #20946). We can't change this without breaking the TpProxy API, sadly.
Comment 1 Xavier Claessens 2012-05-02 04:21:42 UTC
*** Bug 49375 has been marked as a duplicate of this bug. ***
Comment 2 Xavier Claessens 2012-05-02 04:23:34 UTC
As I said on bug #49375:

My personal opinion:
1) yes it should have never been like that.
2) sadly, even in next, that a too subtle behavior change that will lead to
unexpected bugs in our apps and even in tp-glib itself. We have no way to
"grep" all places where we depend on that behavior...
Comment 3 Jonny Lamb 2012-05-10 07:54:01 UTC
I don't think this should block Telepathy 1.0 because, as Xavier says, it's really difficult given problems we'll introduce here are going to be as subtle to find and fix.

Will and I agree that the way to do this is to wait until we start using GDBusProxy or whatever, as client code will be changing anyway. I don't think this is too unreasonable, no?

As a result, I'm going to remove this as a tp-glib-1.0 blocker and WONTFIX this bug.

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.