Bug 49373

Summary:	rethink TpContact<>TpConnection ref cycle
Product:	Telepathy	Reporter:	Xavier Claessens <xclaesse>
Component:	tp-glib	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	xclaesse
Version:	unspecified	Keywords:	patch
Hardware:	Other
OS:	All
URL:	http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=next-contact-conn-refcycle
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	31668
Attachments:	Abort preparing contact-list feature early if CONTACTS iface is missing Small coding style fix TpConnection: Fix leaked result when preparing contact-list feature Rename _tp_contact_connection_invalidated to _disposed TpContact: stop keeping strong ref on its connection Add unit test for TpContact<>TpConnection refcounting NEWS

Description Xavier Claessens 2012-05-02 03:35:05 UTC

From connection.c::tp_connection_invalidated():
  /* Drop the ref we have on all roster contacts, this is to break the refcycle
   * we have between TpConnection and TpContact, otherwise self would never
   * run dispose.
   * Note that invalidated is also called from dispose, so self->priv->roster
   * could already be NULL.
   *
   * FIXME: When we decide to break tp-glib API/guarantees, we should stop
   * TpContact taking a strong ref on its TpConnection and force user to keep
   * a ref on the TpConnection to use its TpContact, this would avoid the
   * refcycle completely. */

Comment 1 Xavier Claessens 2012-06-01 09:40:17 UTC

Created attachment 62384 [details] [review]
Abort preparing contact-list feature early if CONTACTS iface is missing

Current code has issues:
1) It's possible it completes without returning to mainloop first
2) contact-list-state could regress from SUCCESS to FAILED

Comment 2 Xavier Claessens 2012-06-01 09:40:20 UTC

Created attachment 62385 [details] [review]
Small coding style fix

Comment 3 Xavier Claessens 2012-06-01 09:40:24 UTC

Created attachment 62386 [details] [review]
TpConnection: Fix leaked result when preparing contact-list feature

Comment 4 Xavier Claessens 2012-06-01 09:40:27 UTC

Created attachment 62387 [details] [review]
Rename _tp_contact_connection_invalidated to _disposed

That function is called from TpConnection::dispose, not when invalidated

Comment 5 Xavier Claessens 2012-06-01 09:40:31 UTC

Created attachment 62388 [details] [review]
TpContact: stop keeping strong ref on its connection

Reset TpContact::connection to NULL when connection is disposed

Comment 6 Xavier Claessens 2012-06-01 09:40:34 UTC

Created attachment 62389 [details] [review]
Add unit test for TpContact<>TpConnection refcounting

Comment 7 Xavier Claessens 2012-06-01 09:41:28 UTC

patches 1-4 probably needs backporting to master.

Comment 8 Xavier Claessens 2012-06-01 09:57:51 UTC

Created attachment 62390 [details] [review]
NEWS

Comment 9 Xavier Claessens 2012-06-04 04:03:00 UTC

(In reply to comment #7)
> patches 1-4 probably needs backporting to master.

Guillaume reviewed them and they are merged in master already.

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.