After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 666580 - TpContact reference problem.
TpContact reference problem.
Status: RESOLVED OBSOLETE
Product: folks
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: Unset
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2011-12-20 11:06 UTC by Guillaume Desmottes
Modified: 2018-08-04 08:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test app (3.70 KB, text/x-csrc)
2011-12-20 11:06 UTC, Guillaume Desmottes
  Details
contact-chooser: keep a ref on the TpContact we requested (2.40 KB, patch)
2012-01-05 16:15 UTC, Guillaume Desmottes
rejected Details | Review

Description Guillaume Desmottes 2011-12-20 11:06:28 UTC
Created attachment 203939 [details]
test app

Please take a look at the attached test app (you'll have to modify ACCOUNT in order to test it).

It fails because when get_contact_cb() returns, tp-glib release its reference on the TpContact and destroy it.
I know that tpf_persona_dup_for_contact() keeps a weak ref on the contact (I'm the one who suggested this behaviour :) but the persona should ref it as well.

Or am I missing something?
Comment 1 Guillaume Desmottes 2011-12-20 12:27:43 UTC
Looks like the real problem here is that the persona doesn't keep a ref on the TpContact which is really unexpected. But it seems that fixing that isn't a trivial task. :\
Comment 2 Philip Withnall 2011-12-21 20:24:53 UTC
Just to record what was discussed on IRC:

The TpfPersona doesn't keep a strong ref. on the TpContact so that the TpContact can be finalised when tp-glib/Empathy drop their refs on it. This is what causes folks to remove the TpfPersona from the store.

When implementing this, I had assumed that tp-glib would hold a strong reference on the TpContact for the duration of the time that Empathy needed it, but that turns out to not be the case when (e.g.) inviting someone to a chat room. tp-glib drops its ref. to the TpContact as soon as get_contacts() returns.

Suggested solutions which have been mentioned:
 1. Have TpfPersona hold a strong ref. on its TpContact, and instead have TpfPersonaStore hold weak refs. on all its TpfPersonas. Empathy could then hold a strong ref. on the TpfPersona and decide when to finalise it that way. Modifying TpfPersonaStore to use weak refs. on all its personas would be really quite hard and I don't fancy doing it.

 2. Have Empathy hold a strong ref. on the TpContact for the duration of the time it wants the contact to be alive for. I'm not entirely sure of the details, but Guillaume says this would be hacky.

 3. Add a tpf_persona_get_rid_of_me() to complement tpf_persona_dup_for_contact(). This would essentially be the same as option 2, and (from Empathy's point of view) just as icky.


Guillaume, have you had any bright ideas? I haven't. :-(

To summarise: folks is working as I expect it to, but what I expect is different to what Guillaume expects.
Comment 3 Ken VanDine 2011-12-22 21:11:55 UTC
Downstream bug report https://launchpad.net/bugs/907916
Comment 4 Guillaume Desmottes 2012-01-05 16:15:31 UTC
Created attachment 204694 [details] [review]
contact-chooser: keep a ref on the TpContact we requested

Kinda hacky but that's the best we can do without major changes in Folks.
Comment 5 Guillaume Desmottes 2012-01-05 16:16:29 UTC
Comment on attachment 204694 [details] [review]
contact-chooser: keep a ref on the TpContact we requested

Sorry, wrong bug.
Comment 6 Jeremy Whiting 2012-07-16 19:56:39 UTC
Guillaume,

Do you have a preference on which of Philip's suggested solutions would be best?
Comment 7 Philip Withnall 2012-07-16 20:41:46 UTC
Bear in mind that option 1. is essentially an API break, so is not really an option at all.
Comment 8 GNOME Infrastructure Team 2018-08-04 08:32:56 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/folks/issues/37.