Bug 70010

Summary: race condition in async avatar saving can lead to misleading signals
Product: Telepathy Reporter: Simon McVittie <smcv>
Component: tp-glibAssignee: Simon McVittie <smcv>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium CC: xclaesse
Version: git masterKeywords: patch
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 69885    
Attachments: [tp-glib 0.22] TpContact: avoid a race condition in avatar handling
[tp-glib/master] tp_contact_set_attributes: don't warn on genuinely absent interfaces
[tpglib/master] contact_avatar_retrieved: improve debug
[tpglib/master] TpContact: improve debug info for GetContactAttributes

Description Simon McVittie 2013-10-01 17:26:29 UTC
Created attachment 86917 [details] [review]
[tp-glib 0.22] TpContact: avoid a race condition in avatar handling

We have to remember the avatar token synchronously, before we start
async-saving the cached file, so that we can't get into this
situation:

    my avatar is token "A", bytes "AAAA..."
    my avatar changes to token "B", bytes "BBBB..."
    we start saving the file /.../B
    my avatar changes to token "C", bytes "CCCC..."
    saving the file /.../B finishes
    change-notification announces that my avatar has changed to "B"

which is particularly problematic for Mission Control.

---

Regression in Bug #63402.
Comment 1 Simon McVittie 2013-10-01 17:27:41 UTC
Created attachment 86918 [details] [review]
[tp-glib/master] tp_contact_set_attributes: don't warn on genuinely absent  interfaces

The warning message says something like "... supposedly implements
Contacts and Aliasing" but we don't actually check that the connection
claims to implement Aliasing (in either its ContactAttributeInterfaces
or Interfaces). SimplePresence has the same bug.

The Mission Control regression tests trigger this, since they
include some simulated connections with small subsets of interfaces.

---

I think this is too intrusive for 0.22, and it's only a warning anyway. We could delete the warnings altogether in 0.22 if they're a practical problem.
Comment 2 Simon McVittie 2013-10-01 17:27:58 UTC
Created attachment 86919 [details] [review]
[tpglib/master] contact_avatar_retrieved: improve debug
Comment 3 Simon McVittie 2013-10-01 17:28:19 UTC
Created attachment 86920 [details] [review]
[tpglib/master] TpContact: improve debug info for GetContactAttributes
Comment 4 Xavier Claessens 2013-10-01 19:18:39 UTC
Isn't this related/dup of https://bugs.freedesktop.org/show_bug.cgi?id=68149 ?
Comment 5 Simon McVittie 2013-10-02 10:51:10 UTC
(In reply to comment #4)
> Isn't this related/dup of https://bugs.freedesktop.org/show_bug.cgi?id=68149
> ?

Thanks. Yes, partly:

The first patch (for 0.22) fixes this new bug. It's a recent regression (latest version) because I wasn't thinking hard enough about the async avatar saving.

The second patch (for master only, at least for now) fixes Bug #68149.

The others add debug that helped me to solve those.
Comment 6 Guillaume Desmottes 2013-10-02 11:55:12 UTC
Comment on attachment 86917 [details] [review]
[tp-glib 0.22] TpContact: avoid a race condition in avatar handling

Review of attachment 86917 [details] [review]:
-----------------------------------------------------------------

++
Comment 7 Guillaume Desmottes 2013-10-02 11:56:23 UTC
Comment on attachment 86918 [details] [review]
[tp-glib/master] tp_contact_set_attributes: don't warn on genuinely absent  interfaces

Review of attachment 86918 [details] [review]:
-----------------------------------------------------------------

ah good, glad to see this warning being fixed.
Comment 8 Guillaume Desmottes 2013-10-02 11:57:39 UTC
Comment on attachment 86919 [details] [review]
[tpglib/master] contact_avatar_retrieved: improve debug

Review of attachment 86919 [details] [review]:
-----------------------------------------------------------------

++
Comment 9 Guillaume Desmottes 2013-10-02 11:58:42 UTC
Comment on attachment 86920 [details] [review]
[tpglib/master] TpContact: improve debug info for GetContactAttributes

Review of attachment 86920 [details] [review]:
-----------------------------------------------------------------

++
Comment 10 Simon McVittie 2013-10-02 13:57:10 UTC
Fixed in git for 0.22.0; additional patches for Bug #68149 and debug messages in git for 0.23.0. Thanks!

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.