Bug 22119

Summary: [api-break] Proxies should not emit 'invalidated' when disposing.
Product: Telepathy Reporter: Will Thompson <will>
Component: tp-glibAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED WONTFIX QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: xclaesse
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard: ABI-break
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.