Bug 74185 - MC can take its o.fd.T.AccountManager name too early
Summary: MC can take its o.fd.T.AccountManager name too early
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (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: 27727
  Show dependency treegraph
 
Reported: 2014-01-29 14:19 UTC by Simon McVittie
Modified: 2014-01-29 14:42 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Comment that the first bus name we take is an initialization mutex (1.13 KB, patch)
2014-01-29 14:22 UTC, Simon McVittie
Details | Splinter Review
McdAccountManager: separate setup lock for AM from lock for new accounts (1.13 KB, patch)
2014-01-29 14:23 UTC, Simon McVittie
Details | Splinter Review
McdAccountManager: separate setup lock for AM from lock for new accounts (10.32 KB, patch)
2014-01-29 14:24 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2014-01-29 14:19:07 UTC
McdLoadAccountsData was being used for two separate things: either
the initial setup process for the AccountManager (not specific to an
account, blocking taking the AccountManager name) or the setup process
for an account being added by a storage plugin later on (specific to
an account, and hopefully after the AccountManager name has been taken).
    
Since release_load_accounts_lock() takes the AccountManager name in an
idempotent way, this happened to work most of the time. However,
if a storage plugin signals the addition of an account not in the
initial batch, and adding that account finishes before the setup
process for the initial batch, we'd take the name too early, and have
our D-Bus API before we should - that'd be bad.
    
Redo this so it makes more sense. We don't actually need a struct
for the "initial batch" case, because the McdAccountManager and maybe
a MigrateCtx is enough; just keep a count of locks in
McdAccountManagerPrivate.
Comment 1 Simon McVittie 2014-01-29 14:22:42 UTC
Created attachment 93002 [details] [review]
Comment that the first bus name we take is an  initialization mutex

We need a mutex that will protect our account migration process
from other copies of MC running in parallel. If we don't take the
"main" name until later, then that name can't be our mutex.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=24000
Comment 2 Simon McVittie 2014-01-29 14:23:21 UTC
Created attachment 93003 [details] [review]
McdAccountManager: separate setup lock for AM from lock  for new accounts

---

See Comment #0 for full commit message :-)
Comment 3 Simon McVittie 2014-01-29 14:24:04 UTC
Created attachment 93004 [details] [review]
McdAccountManager: separate setup lock for AM from lock  for new accounts

---

Oops, wrong file. Try this one. Again, Comment #0 is the long commit message.
Comment 4 Guillaume Desmottes 2014-01-29 14:27:40 UTC
Comment on attachment 93002 [details] [review]
Comment that the first bus name we take is an  initialization mutex

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

++
Comment 5 Guillaume Desmottes 2014-01-29 14:29:43 UTC
Comment on attachment 93004 [details] [review]
McdAccountManager: separate setup lock for AM from lock  for new accounts

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

++
Comment 6 Simon McVittie 2014-01-29 14:42:14 UTC
Fixed in git for 5.17.0. I'm not sure about this for 5.16, so I'll let it get some testing first.


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.