Bug 70010 - race condition in async avatar saving can lead to misleading signals
Summary: race condition in async avatar saving can lead to misleading signals
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks: 69885
  Show dependency treegraph
 
Reported: 2013-10-01 17:26 UTC by Simon McVittie
Modified: 2013-10-02 13:57 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
[tp-glib 0.22] TpContact: avoid a race condition in avatar handling (2.96 KB, patch)
2013-10-01 17:26 UTC, Simon McVittie
Details | Splinter Review
[tp-glib/master] tp_contact_set_attributes: don't warn on genuinely absent interfaces (9.42 KB, patch)
2013-10-01 17:27 UTC, Simon McVittie
Details | Splinter Review
[tpglib/master] contact_avatar_retrieved: improve debug (1.34 KB, patch)
2013-10-01 17:27 UTC, Simon McVittie
Details | Splinter Review
[tpglib/master] TpContact: improve debug info for GetContactAttributes (6.41 KB, patch)
2013-10-01 17:28 UTC, Simon McVittie
Details | Splinter Review

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.